buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26309, Implement SOF_BROADCAST

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #26309: Implement SOF_BROADCAST

Submitter:  Frédéric Bernon <fbernon>
Submitted:  Thu 23 Apr 2009 01:10:55 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Feature Request Status:  Fixed
Privacy:  Public Assigned to:  fbernon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Wed 29 Apr 2009 12:49:34 PM UTC, comment #21: 

I have check in a first patch. Since this patch work on all API layers, I have name options IP_SOF_BROADCAST & IP_SOF_BROADCAST_RECV (since others IP_ levels options are prefixed like this).

These options are disabled as default values in opt.h. The default behavior is not change, so, it's compatible with previous releases.

IP_SOF_BROADCAST_RECV=1 requires IP_SOF_BROADCAST=1 (a check is done in init.c).

SO(F)_BROADCAST comments in ip.h & sockets.h tell to check these options.

Implement IP_RECEIVE_BROADCAST could be done as another task I think. The problem is to find something which is not too "windows-like".

If there is no comments, I will chose this task as done.


Frédéric Bernon <fbernon>
Group Member
Sun 26 Apr 2009 05:59:32 PM UTC, comment #20: 

I do not really have a preference, but I also don't like to move away from the standard just to prevent breaking older applications running on lwIP.

We should make one thing clear: are we talking about the socket API only (for which these flags are) or about the raw API, too?

We could implement these send-limitation at socket-level. The receive-limitation will have to go into the raw API.

However, how do we implement this with only one setsockopt flag if the default should be "receive broadcast but don't send"? For your scenario, you would have to turn of receiving broadcast, but since SOF_BROADCAST is off by default, you cannot use that flag for it.

On the other hand, since there does not seem to be a standard option to turn off receiving broadcast, maybe we should go the easiest way and let SOF_BROADCAST influence RX and TX (but RX only when LWIP_SO_BROADCAST_RECV==1) and let it default to on?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 26 Apr 2009 05:52:35 PM UTC, comment #19: 

One option in opt.h will define the behavior of SO(F)_BROADCAST on recv operations we want. Two options (like I proposed) avoid to break any application.

But if you want, we wan add something like IP_RECEIVE_BROADCAST.

I have a small preference on the opt.h option, because due to the lwIP resources pool, it could help to indicate a potential problem to port maintainers (I suppose their read more the opt.h than the CHANGELOG, but perhaps I'm wrong).

Tell me if you have a real preference, both solutions are ok for me.

Frédéric Bernon <fbernon>
Group Member
Sun 26 Apr 2009 05:33:24 PM UTC, comment #18: 

I must say I don't really favor adding too many options. Do we really need this option? Can't that be achieved by some flag for setsockopt like MS does it? That way we could prevent having yet another configuration option for this...

And what would be the default? The current implementation (meaning always receive and send broadcast)? I think the default (according to the standard) should be "receive broadcast but don't send"?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 26 Apr 2009 03:32:39 PM UTC, comment #17: 

yes, in my case, I prefer a LWIP_SO_BROADCAST_RECV opt.h option saying the scope of the SO(F)_BROADCAST flag.

if there is no objections, I will check in a such change tonight or tomorrow...

Frédéric Bernon <fbernon>
Group Member
Sun 26 Apr 2009 02:51:24 PM UTC, comment #16: 


> Isn't that a bit unportable: your socket would get unusable if > the interface address changed...


yes, I agree, it's not very handy.

>However, the opengroup documentation seems to say the contrary >of what's implemented in linux and windows. I really don't know >what's the best thing to do here, but normally I'd vote for the >opengroup definition...


I browsed the FreeBSD sources, and only sending is implemented.
If you google on this subject, it seems that things are very unclear about broadcast reception, including on win32. It seems to depend on windows versions.
The MSDN lib also mentions a IP_RECEIVE_BROADCAST option:
http://msdn.microsoft.com/en-us/library/ms738586(VS.85).aspx
On this page, they talk only about sending:
http://msdn.microsoft.com/en-us/library/ms740532(VS.85).aspx

>LWIP_SO_BROADCAST for the send operations, and >LWIP_SO_BROADCAST_RECV for the recv operations.
>Both disabled as default values to get a source code >compatibility with current behavior.
>Like this, each one could choose. About binding on only one >interface, I share Simon's point of view.


Seems fine to me.

We could also implement IP_RECEIVE_BROADCAST... but it's windows specific, so I have the feeling you won't agree ;-)

Stephane Lesage <slesage>
Sun 26 Apr 2009 12:37:41 PM UTC, comment #15: 

The best seems to get some opt.h options:

LWIP_SO_BROADCAST for the send operations, and LWIP_SO_BROADCAST_RECV for the recv operations. Both disabled as default values to get a source code compatibility with current behavior.

Like this, each one could choose. About binding on only one interface, I share Simon's point of view.


Frédéric Bernon <fbernon>
Group Member
Sun 26 Apr 2009 11:23:46 AM UTC, comment #14: 

On lwip-devel, Stéphane Lesage brought up this link:
http://lkml.indiana.edu/hypermail/linux/net/0703.0/0024.html

plus:

> So if you don't want to receive broadcast traffic,
> you should not bind your socket to IPADDR_ANY,
> but to your interface IP address.


Isn't that a bit unportable: your socket would get unusable if the interface address changed...

However, the opengroup documentation seems to say the contrary of what's implemented in linux and windows. I really don't know what's the best thing to do here, but normally I'd vote for the opengroup definition...

Simon Goldschmidt <goldsimon>
Group administrator
Fri 24 Apr 2009 12:31:31 PM UTC, comment #13: 

Stéphane, about your question :

http://linux.die.net/man/7/socket
http://msdn.microsoft.com/en-us/library/ms740476(VS.85).aspx

(PS: please, reply using the bugtracker)

Frédéric Bernon <fbernon>
Group Member
Fri 24 Apr 2009 10:10:47 AM UTC, comment #12: 

For send operations, changes are :

in udp_sendto_if (at the begining):
  /* broadcast filter? */
  if ( ((pcb->so_options & SOF_BROADCAST) == 0) && ip_addr_isbroadcast(dst_ip, netif) ) {
    LWIP_DEBUGF(UDP_DEBUG | 1, ("udp_sendto_if: SOF_BROADCAST not enabled on pcb %p\n", (void *)pcb));
    return ERR_VAL;
  }

in raw_sendto, after the ip_route checking:

   /* broadcast filter? */
  if ( ((pcb->so_options & SOF_BROADCAST) == 0) && ip_addr_isbroadcast(ipaddr, netif) ) {
    LWIP_DEBUGF(RAW_DEBUG | 1, ("raw_sendto: SOF_BROADCAST not enabled on pcb %p\n", (void *)pcb));
    /* free any temporary header pbuf allocated by pbuf_header() */
    if (q != p) {
      pbuf_free(q);
    }
    return ERR_VAL;
  }


Next, some udp clients need to add pcb->so_options|=SOF_BROADCAST; after udp_new : there is dhcp.c, and netbios.c

Frédéric Bernon <fbernon>
Group Member
Fri 24 Apr 2009 08:53:26 AM UTC, comment #11: 

I attach a patch for recv operations. It is based on raw_input & udp_input.

Comments ?


(file #18003)

Frédéric Bernon <fbernon>
Group Member
Fri 24 Apr 2009 07:54:58 AM UTC, comment #10: 

I think SO_BROADCAST shouldn't affect TCP connections, so ip_input might not be the best.

Thanks for clarifying what SO_BROADCAST does: after reading comment #6 I think it's better not to worry about shutdown for now.

Kieran Mansley <kieranm>
Group Member
Fri 24 Apr 2009 06:04:25 AM UTC, comment #9: 

I'd only favour ip_input because we can avoid having duplicate code and the filter is based on IPs, not ports (hence ip_input would be the correct filter level).

Simon Goldschmidt <goldsimon>
Group administrator
Thu 23 Apr 2009 07:46:27 PM UTC, comment #8: 

I think that the change in raw_input and in udp_input (sorry for the mistake in the previous post) is better, since inside ip_input, we don't manage pcbs, but we did in raw_input and in udp_input.

Frédéric Bernon <fbernon>
Group Member
Thu 23 Apr 2009 04:09:58 PM UTC, comment #7: 

ip_input vs (raw_sendto + udp_sendto_if), I will check where is the best place.

Frédéric Bernon <fbernon>
Group Member
Thu 23 Apr 2009 04:01:46 PM UTC, comment #6: 

I thought SOF_BROADCAST would say "don't accept broadcast data", not "don't queue received data for RX"? If so, those 2 features are not that similar to me. They both do RX filtering, but with different conditions.

However, I'd implement that in ip_input, not in the API layer: I think it's a desireable flag for all applications, even when using the RAW API.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 23 Apr 2009 03:00:40 PM UTC, comment #5: 

OK, it wasn't quite clear!

I meant that there is a certain amount of overlap here between SO_BROADCAST and SHUT_RD.  I don't think we can use one to implement the other, but that they both require (among other things) the ability to say "don't queue received data for RX".  I just think it would make sense to have that bit of the code generic if we can.

Kieran Mansley <kieranm>
Group Member
Thu 23 Apr 2009 02:52:56 PM UTC, comment #4: 

Implement SOF_BROADCAST is a "new" feature which will avoid some bugs.

Using shutdown is not a so good solution, since I remember that  calling "shutdown(sock,SHUT_RD)" will return an error on any next "recv/recvfrom" calls, but packets continue to be enqueued: I'm sure of that with windows system (see http://msdn.microsoft.com/en-us/library/ms740481(VS.85).aspx), but not for linux (it's not very clear). More, "shutdown" should do some extras operations on tcp sockets (like send a FIN after all data are sent and acknowledged by the receiver).


Frédéric Bernon <fbernon>
Group Member
Thu 23 Apr 2009 02:43:36 PM UTC, comment #3: 

OK, I understand now: I didn't realise the traffic was coming from some other application and for some other device. 

I would like to see this implemented in the stack with a more generic flag than SO_BROADCAST, and have SO_BROADCAST set that flag to get the correct behaviour.  If it's done in a generic way then we'll be able to use the same flag to implement shutdown() for reading for example.  Does that make sense?

Kieran Mansley <kieranm>
Group Member
Thu 23 Apr 2009 02:32:51 PM UTC, comment #2: 

The problem is I received in my lwIP device's socket some broadcast traffic between another application (not mine) and another device (not mine).

In a classical system (windows or linux), don't have the SO_BROADCAST option enabled on a socket will avoid to receive and queue such broadcast traffic where the "dest" port is the same than my socket local one (this is a unlucky case, but this case happens on the field).

>For a normal socket, not reading from it when it is receiving data is essentially a bug in the application


It's not, if the socket is only use to send packets, why do you want I do any extra recvfrom ? Here a classical socket "sender" (without any checking for the explain = socket( AF_INET, SOCK_DGRAM, 0);

memset( &saLocal, 0, sizeof(saLocal));
saLocal.sin_family      = AF_INET;
saLocal.sin_port        = htons(INADDR_ANY);
saLocal.sin_addr.s_addr = htonl(INADDR_ANY);

bind( ivp->hSocket, (struct sockaddr *)&saLocal, sizeof(saLocal));

namelen = sizeof(saLocal);
getsockname( hSocket, (struct sockaddr *)&saLocal, &namelen);
printf("saLocal.sin_port==%lu\n", (unsigned long)ntohs(saLocal.sin_port));

while(1) {
  sendto(...)
}

In a such code, the socket only send data (rtp packets in my case). The getsockname show that the local port is 4096. In the case I described, any broadcast traffic with dest to 4096/udp will be queued on lwIP (since even if SO_BROADCAST is not set, packets are enqueued since the option is not implemented). On Linux or Windows, you will not have such problem since their IP stacks implement SO_BROADCAST: so, since the option is not set on the socket, broadcast packets are dropped by the stack...

Since lwIP use a common poll of resources for all sockets, this case can stop the stack to work (by example, ARP packets will stop to be received, with all the bad consequences you can guess).

Of course, in raw api, since all incoming packets can be processed by the recv callback, it's different. But with sockets, we should not have to call lwip_recvfrom + MSG_DONTWAIT to flush any incoming traffic: doing a such workaround is not a "nice" solution.

I will write a patch file to explain how I see the solution. I think that only some lines are needed...


Frédéric Bernon <fbernon>
Group Member
Thu 23 Apr 2009 01:35:33 PM UTC, comment #1: 

Can you explain how implementing SO_BROADCAST will solve the problem of the stack buffering data on RX?

For a normal socket, not reading from it when it is receiving data is essentially a bug in the application, but its one that has particularly bad effects on lwIP.  I'm trying to understand how SO_BROADCAST makes a difference.

Kieran Mansley <kieranm>
Group Member
Thu 23 Apr 2009 01:10:55 PM UTC, original submission:  

I propose to implement SOF_BROADCAST option. I post that like a bug because without it, we could block the stack some cases. I explain:

On my network, one application use a "IP device detection" like this: the application send a "discover device" udp message in broadcast, the device send its response with a broadcast on the application's "source" udp port. Like this, the device can be detected even if the application and the device are not on the same ip subnet. In my domain (IP video devices), lot of manufacturers do the same thing.

The problem is the response: since the application create its socket with "any" as port, most of time, the source port is sometimes 4096/udp. On my lwIP device (which is not managed by this application), I got a udp socket which only send udp packets, so, it is also binded with "any" port, and lwIP give it... 4096 (which is defined by UDP_LOCAL_PORT_RANGE_START).

Since I never "read" the socket (the protocol doesn't need it), each time that the other device send its response to 255.255.255.255:4096, my lwIP device receive the packet, and this one is queued. Since I never read the socket, and since my mailboxes are not really limited, all my PBUF_POOL (the type I use for incoming packets) are queued, and after several months (that's why it was difficult to find the problem), all resources are queued in my mailbox, and the stack stop to work, due to the fact that don't have any PBUF_POOL for incoming packets.

I thought, and it was my mistake, that SO_BROADCAST (SOF_BROADCAST in RAW API), was implemented. When I look in sockets.h, its indicated "Unimplemented", but the setsockopt doesn't return ENOPROTOOPT (since the code really put the SOF_BROADCAST flag in pcb.ip::so_options).

Since my lwip socket never set the option SO_BROADCAST, I thought I will never got this problem. In most of systems (windows & linux), if you don't have this option on a socket, it disable the broadcast on send and recv operations. I now that http://www.opengroup.org/pubs/online/7908799/xns/setsockopt.html only talks about the "send", but not linux pages and MSDN ones talk about both operations.

One workaround could be to use SO_RCVBUF option to have a "0 bytes" receive buffer size. Another one is to patch my embedded code to be sure that all my sockets flush any incoming packets (with a lwip_recvfrom + MSG_DONTWAIT).

But I thought it's better to implement this SOF_BROADCAST checking in raw_input and in udp_input (at the level, the option would be valid for all lwIP API). For the send, it could be done in raw_sendto and udp_sendto_if.

The only problem I see is that implement this option will change the current lwIP behavior for application using broadcast, but I think it's better than stay like this. Else, we could implement a LWIP_SOF_BROADCAST option in opt.h.

Of course, in my case, I could got the same kind of problem if "someone" send on this socket unicast udp packets. But it's a more a "security" case, than a "stability" one.

Since the change will be small, I think to check in it in the current CVS.

Thoughts ?








Frédéric Bernon <fbernon>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #18003:  SOF_BROADCAST.txt added by fbernon (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by slesage (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-05-01 fbernon StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2009-04-29 kieranm StatusWorks For Me Ready For Test
    2009-04-29 fbernon StatusNone Works For Me
    2009-04-24 fbernon Attached File- Added SOF_BROADCAST.txt, #18003

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code