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

Add in support for network namespaces #676

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

acmay
Copy link

@acmay acmay commented Feb 22, 2018

Here is a cut at doing the namespace changes in libpcap for issue #587

I am still not sure on the libpcap details and I am tempted to create a new function pointer to set/unset the namespace (or not) from the pcap.c file so I don't have to catch every return location in the -linux files.

I also left a few comment questions.

@mcr mcr added this to the release-after-next milestone Apr 26, 2019
@mcr
Copy link
Member

mcr commented Apr 26, 2019

please rebase.

@acmay
Copy link
Author

acmay commented Apr 26, 2019

Still working on it. Got it building and starting to work just now.

Any ideas on the handle.opt.device vs handlep->device?
I did a search replace on handle.opt.device to HDEV(handle) but still looking if there is a different approach.

@fxlb
Copy link
Member

fxlb commented May 1, 2019

@acmay "wants to merge 774 commits" ???
Have you rebased your branch on the-tcpdump-group:master ?

@acmay
Copy link
Author

acmay commented May 1, 2019

@acmay "wants to merge 774 commits" ???
Have you rebased your branch on the-tcpdump-group:master ?

Just struggling with getting git to do what I want. So no I don't want to merge all those commits just one. I can't tell if I can fix it in git easily enough or if I should just start over with doing some file copies.

acmay added 2 commits May 1, 2019 13:43
Add in cmake checking for namespace support and pcap_get_caps()

Fixup pcap_get_caps() function to let an application find out the caps
that need to be kept to capture packets.

Add in c-define for SYS_CAPABILITY

Add in another ifdef check on sys capabilities

Yet another spot for an ifdef namespaces

Always pass in devprefix instead

Remove un-needed var
@acmay
Copy link
Author

acmay commented May 2, 2019

Hopefully closer to ready now. Some whitespace fixes to make but I would like to get some comments before taking another pass and getting things down to 1 commit.

@fxlb
Copy link
Member

fxlb commented Aug 4, 2019

This PR should be rebased on master for a better history.

@omerb4
Copy link

omerb4 commented Jan 15, 2020

Is this PR going to be merged soon?

@mcr
Copy link
Member

mcr commented Jan 15, 2020 via email

@mcr mcr self-assigned this Jan 15, 2020
@fxlb
Copy link
Member

fxlb commented Jan 15, 2020

Github message: "This branch has conflicts that must be resolved".
This PR should be rebased on the-tcpdump-group:master for a better history and to fix conflicts.

@omerb4
Copy link

omerb4 commented Jan 15, 2020

Will this fix allow me to use it for docker namespace?
Cause the default namespace path is: /var/run/netns while in docker is: /var/run/docker/netns

@mcr
Copy link
Member

mcr commented Jan 16, 2020 via email

@acmay
Copy link
Author

acmay commented Jan 17, 2020

I think I can spend some time rebasing and looking at things soon here.

Will this fix allow me to use it for docker namespace?
Cause the default namespace path is: /var/run/netns while in docker is: /var/run/docker/netns

I haven't used docker much myself to test there.
I would expect things to run best outside of docker. I would think inside of docker it be yet another possible level of namespaces.
Can you do a "strace ip netns ls" and see how it does it? And if you create another namespace inside docker does it appear in the to host OS layer?
I can try to check the iproute2 source to see if there are any tricks there.

@acmay
Copy link
Author

acmay commented Jan 17, 2020

iproute2 just looks in one dir.
https://github.com/shemminger/iproute2/blob/master/lib/namespace.c#L119

It is only a compile time option to change it and I am not sure what each distro does to build it. But we could do a build time option to at least match iproute2.
iproute2/iproute2@e2f5cec

@omerb4
Copy link

omerb4 commented Jan 18, 2020

Build time option like in iproute2 sounds good to me

@omerb4
Copy link

omerb4 commented Mar 1, 2020

Any news here?

@mcr mcr modified the milestones: release-after-next, next-release Mar 2, 2020
@guyharris
Copy link
Member

Solaris's zones (see, for example, this Sun paper and this article) might fit into a similar framework.

So, for the "enumerate devices" API, would it make more sense to

  1. have it enumerate interfaces in all namespaces if running in the root namespace/global zone and enumerate devices in the current namespace in other namespaces/zones

or

  1. take a namespace/zone name as an argument and, if possible, enumerate interfaces in that zone, with NULL meaning "current namespace/zone" - and have another API to list namespaces/zones, for the benefit of GUI sniffer applications? (Another API could return the appropriate name for what's being enumerated there - "namespace" or "zone".)

For zones, we already support {zone}/{interface} in Solaris for the "create" API, so presumably something similar would be used for Linux (perhaps ":" should be replaced by "/", for consistency).

(See this blog post, too.)

check_include_file(sys/capability.h HAVE_SYS_CAPABILITY_H)
if(HAVE_SYS_CAPABILITY_H)
add_definitions(-DHAVE_SYS_CAPABILITY=1)
check_c_source_compiles(

Choose a reason for hiding this comment

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

I think you can HAVE_LINUX_NETWORK_NAMESPACE without HAVE_SYS_CAPABILITY_H, no?

@@ -303,6 +303,38 @@ include(CheckTypeSize)
check_include_file(inttypes.h HAVE_INTTYPES_H)
check_include_file(stdint.h HAVE_STDINT_H)
check_include_file(unistd.h HAVE_UNISTD_H)
check_include_file(sys/capability.h HAVE_SYS_CAPABILITY_H)
if(HAVE_SYS_CAPABILITY_H)
add_definitions(-DHAVE_SYS_CAPABILITY=1)

Choose a reason for hiding this comment

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

Shouldn't this be true only if HAVE_SYS_CAPABILITY (set below on lines 326-336 is true?

@tr33oph
Copy link

tr33oph commented Apr 6, 2023

Any news here?

@irozzo-1A
Copy link

@acmay I'm interested in this feature, is there the will to carry through? I don't know the code base at the moment, but I could try to help if needed.

cc @omeranson

@acmay
Copy link
Author

acmay commented May 24, 2023 via email

@irozzo-1A
Copy link

I just haven’t found the time to do it myself so any help would be great. I also didn’t know the code base going into it and the auto builders are helpful to do the different build variants.

On Wed, May 24, 2023, at 1:49 AM, Iacopo Rozzo wrote: @acmay https://github.com/acmay I'm interested in this feature, is there the will to carry through? I don't know the code base at the moment, but I could try to help if needed. cc @omeranson https://github.com/omeranson — Reply to this email directly, view it on GitHub <#676 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANUHBQAHCI55USS4SXC4TLXHXDSHANCNFSM4ER3KSSQ. You are receiving this because you were mentioned.Message ID: @.***>

Thanks @acmay, I will start familiarizing myself with the codebase and then I could try to rebase your PR.
Was there anything missing apart from the few unhandled comments?

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

Successfully merging this pull request may close these issues.

None yet

8 participants