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

No need to be root for pcapplay. #57

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

No need to be root for pcapplay. #57

wants to merge 2 commits into from

Conversation

wdoekes
Copy link
Member

@wdoekes wdoekes commented Mar 12, 2014

The first part, needed to get pcapplay without root powers going.

@wdoekes
Copy link
Member Author

wdoekes commented Mar 12, 2014

Ok. And pushing along, the second part got appended to this pull request too.

Oh, well. That was the intention anyway.

@rkday
Copy link
Contributor

rkday commented Mar 14, 2014

Looks good, thanks! I'd like to compile this and kick the tyres myself before I merge, but I should get to that this week.

What OSes have you tested this on? I have Linux, OS X, Cygwin and FreeBSD available to me, and it probably makes sense for me to test on the ones you haven't been able to.

@wdoekes
Copy link
Member Author

wdoekes commented Mar 15, 2014

Only tested on Linux.

Similar changes have been running for a longer while on the sipp-osso hg fork, but those too have only been seen by a Linux OS.

Cheers,
Walter

(Excuse the short msg. This was composed on a phone...)

Rob Day notifications@github.com wrote:

Looks good, thanks! I'd like to compile this and kick the tyres myself before I merge, but I should get to that this week.

What OSes have you tested this on? I have Linux, OS X, Cygwin and FreeBSD available to me, and it probably makes sense for me to test on the ones you haven't been able to.


Reply to this email directly or view it on GitHub.

@wdoekes wdoekes mentioned this pull request May 13, 2014
@wdoekes
Copy link
Member Author

wdoekes commented May 13, 2014

I believe this has one issue: if you're using RTCP and/or other data over multiple ports, this does not work.

Not sure how to proceed.

  • We could run the old code if one is root.
  • Or we could add code to open extra ports as needed.

Both have disadvantages:

  • Using the old root-code makes behaviour inconsistent/unpredictable.
  • Opening ports as needed may not be possible (someone may have already allocated those).

@rkday
Copy link
Contributor

rkday commented May 28, 2014

Sorry for the delay - had to mull that issue over. I think the right thing to do is to open the necessary range of ports - but we should do that at the very start (by checking the pcap files, seeing how many unique port numbers there are, and opening that many at the right offsets from the "base" port). If we fail to open one, we can just choose another "base" port and retry.

@wdoekes wdoekes force-pushed the master branch 2 times, most recently from bebdf78 to 8923379 Compare October 3, 2014 06:55
@wdoekes wdoekes mentioned this pull request May 5, 2015
@vodik
Copy link
Member

vodik commented May 29, 2015

Another option is to set CAP_NET_RAW on sipp.

@wdoekes
Copy link
Member Author

wdoekes commented Jun 1, 2015

An interesting option, but it has its own drawbacks.

I tried it indeed:

sudo setcap cap_net_raw+ep `which sipp`

But that broke my unittest framework, which introspects which sockets are open by looking at proc:

$ ls /proc/`pidof sipp`/fd
ls: cannot open directory /proc/9986/fd: Permission denied

@vodik
Copy link
Member

vodik commented Jun 1, 2015

Your personal unittest stuff Linux? Maybe ss would be more appropriate. The -p flag adds process information to the output.

@wdoekes
Copy link
Member Author

wdoekes commented Jun 1, 2015

Same difference:

$ ss -lp | grep 5060
tcp    UNCONN     0      0                    *:ipproto-5060               *:*        users:(("sipp",3600,4))

Vs:

$ ss -lp | grep 5060
tcp    UNCONN     0      0                    *:ipproto-5060               *:*

Was to be expected ;)

@vodik
Copy link
Member

vodik commented Jun 1, 2015

Yeah, huh. TIL.

@vodik vodik force-pushed the master branch 3 times, most recently from 231edd4 to 341fb97 Compare July 16, 2015 20:58
@wdoekes
Copy link
Member Author

wdoekes commented Aug 30, 2015

Interestingly, the polycom-sipped also did some work on this, seeing that the RAW sockets changed to DGRAM in this commit:
SIPp/polycom-sipped@e9b1f62

@alienpenguin
Copy link

hi, is there any update on this PR?

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