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

set SO_BROADCAST on the divert socket. #249

Closed
wants to merge 3 commits into from
Closed

Conversation

ndenev
Copy link
Contributor

@ndenev ndenev commented Dec 22, 2012

set SO_BROADCAST on the divert socket so that broadcast packets can be reinjected.

flag = 1;

if (setsockopt(nq->fd, SOL_SOCKET, SO_BROADCAST, &flag, sizeof(flag)) == -1) {
SCLogWarning(SC_WARN_IPFW_SETSOCKOPT,"Can't set IPFW divert socket broadcast flag: %s", strerror(errno));
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be a warning if it's actually an error, which it seems to be since we return "failed"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sent from my iPhone

On 22.12.2012, at 14:00, Victor Julien notifications@github.com wrote:

In src/source-ipfw.c:

@@ -352,6 +353,15 @@ TmEcode ReceiveIPFWThreadInit(ThreadVars _tv, void *initdata, void *_data)
SCReturnInt(TM_ECODE_FAILED);
}

  • /* set SO_BROADCAST on the divert socket, otherwise sendto()
  • \* returns EACCES when reinjecting broadcast packets. */
    
  • flag = 1;
  • if (setsockopt(nq->fd, SOL_SOCKET, SO_BROADCAST, &flag, sizeof(flag)) == -1) {
  •    SCLogWarning(SC_WARN_IPFW_SETSOCKOPT,"Can't set IPFW divert socket broadcast flag: %s", strerror(errno));
    

This shouldn't be a warning if it's actually an error, which it seems to be
since we return "failed"

I just copy+pasted from the setsockopt() call before this one, so maybe
both need to be changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, changing both would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the pull request with another commit changing them both to errors instead of warnings.

so treat them as such and print error instead of warning.
SC_ERR_IPFW_NOPORT,
SC_WARN_IPFW_RECV,
SC_WARN_IPFW_XMIT,
SC_WARN_IPFW_SETSOCKOPT,
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the actual numbers of the error codes, which is unwanted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I leave the WARN and add the ERR at the end?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we shouldn't change the order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. They are not going to be grouped anymore, but the order will remain the same for the existing error codes. I will push the update now.

restore SC_WARN_IPFW_SETSOCKOPT
move SC_ERR_IPFW_SETSOCKOPT at the end of the enum
@inliniac
Copy link
Contributor

inliniac commented Jan 9, 2013

Merged, thanks!

@inliniac inliniac closed this Jan 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants