Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement dedicated interface (for Linux) #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bictorv
Copy link
Contributor

@bictorv bictorv commented Feb 18, 2018

The new stuff in osdnet seems to have made it impossible(?) to use dedicated interfaces, since the only options are tun/tap/pcap but no "raw access". This branch takes code from the Panda KLH10_NET_LNX case which makes that easily possible again.

Someone should review this e.g. to see if it can be auto-enabled; I only verified that it works with
autogen.sh --enable-lnx
and
the "ifmeth=lnx dedic=true" case for ni20.

And maybe someone can tell me this wasn't necessary because some case of the old code worked just as well?

@Rhialto
Copy link
Member

Rhialto commented Feb 18, 2018

I have to admit that I never tried the dedicated interface option, since I didn't have one to spare or anything set up to connect with it.
But it was my expectation that it could/would still work. The pcap library is basically a front-end for bpf and the other packet filters from different Unixen. Since these were used in combination with dedic=true, I would hope that it still works with pcap. But there might some detail here that I overlooked, of course.
Now if it turns out that separate code is indeed necessary, at least it is more modular than in the past :-)


pfdata->pf_meth = PF_METH_LNX;
pfdata->pf_read = osn_pfread_fd;
pfdata->pf_write = osn_pfwrite_fd;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit: The definition of osn_pfread_fd and osn_pfwrite_fd are #ifdefed with a different condition. When configuring TUN and TAP out it will be missing. Easy to fix of course.

@bictorv
Copy link
Contributor Author

bictorv commented Feb 18, 2018

Arrgh. It seems you're right about pcap working with dedic=true. Could this still be useful in case pcap isn't around? Or maybe I just wasted a few hours.

@Rhialto
Copy link
Member

Rhialto commented Feb 18, 2018

It might in principle be useful if pcap is lacking. But my hope is that it is basically always available.
So I'm not actively against merging it but it ought to be unneeded.
Lets see if there are other opinions.

@larsbrinkhoff
Copy link
Member

I'll just add that I'm following this with interest, but I don't have anything useful to add regarding technical details.

@niallor
Copy link

niallor commented Jul 21, 2018

This gives me some interesting hints.

In case it's of interest for testing (Rhialto: "... I never tried the dedicated interface option, since I didn't have one to spare or anything set up to connect with it."), I'll just mention how I've been doing my plumbing until now.

I've been using DEDIC=TRUE with the Panda binaries running in an Ubuntu VM under VirtualBox on a macOS laptop. VirtualBox allows multiple virtual NICs to be configured on a guest; a Bridged mode allows one of these to be plumbed through to one of the laptop's external interfaces. In my configuration, this appeared to the Ubuntu guest as enp0s9. With Panda, I was able to use

 devdef ni0 564 ni20 dedic=TRUE ifc=enp0s9

and see the (klt20) KN 'just work', once I had the correct configuration in the relevant SYSTEM: files. I could both telnet into the KN and reach a remote FTP server from the KN.

Due to the Panda KN locking up in certain circumstances, apparently in a tight loop, I'm now trying to make a freshly-built KN from this repo work in my environment, and finding a surprising number of difficulties, including behaviour which seems inconsistent with the documentation; I'll write these troubles up after I've put some more time and effort into following the hints here. I'll likely try pcap first, and then lnx.

@larsbrinkhoff
Copy link
Member

@niallor,

We have seen what is perhaps a similar lock-up when building ITS with KLH10. We don't know why, but it looks like the PDP-10 will stop accepting console input.

The new stuff in osdnet seems to have made it impossible(?) to use
dedicated interfaces, since the only options are tun/tap/pcap but no
"raw access". This branch takes code from the Panda KLH10_NET_LNX case
which makes that easily possible again.
@larsbrinkhoff
Copy link
Member

@bictorv & @Rhialto,

Any thoughts on this pull request?

I forcibly rebased the branch on top of the latest commit and fixed some conflicts.

@bictorv
Copy link
Contributor Author

bictorv commented Jun 17, 2019

On one hand, it's better to have a smaller code base (i.e. only the pcap implementation, which somebody else maintains), on the other maybe there could be bugs in one of them?

@Rhialto
Copy link
Member

Rhialto commented Jun 18, 2019

I am not dead-set against it, but there are messages above that dedic=true appears to work, which seems to make it an unneeded addition.

Maybe with this we reach a point where we could make some sort of plugin implementation? Then extra networking implementations won't add so much complexity any more. There is already, in effect, a sort of API for such a plugin.

Or am I now overdoing it?

@larsbrinkhoff
Copy link
Member

That does sound to me like a bit too much effort for changing something that already works quite well.

@larsbrinkhoff
Copy link
Member

As for the matter at hand in this pull request, I am myself a big fan of reusing other people's code. I guess that's some kind of vote in favour of pcap. I could even imagine dropping tun/tap if pcap provides all the necessary features. But again, if it ain't broke don't fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants