search

linux-can / socketcand

Server to access CAN sockets over ASCII protocol
165 stars 41 forks source link

Fix incorrect broadcast address #26

Closed faisal-shah closed 1 year ago

faisal-shah commented 1 year ago

On at least some linux systems, the broadcast address is unset (0.0.0.0). Firstly, this is incorrect. Secondly it is not allowed per RFC1122 3.2.1.3 (a): https://www.rfc-editor.org/rfc/rfc1122#section-3

The broadcast address is computed from the ip address, and net mask. broadcast = ip | ~net_mask

This remains the correct vale of the broadcast address regardless of what SIOCGIFBRDADDR returns. As it will be garbage if no one ever set it (with SIOCSIFBRDADDR perhaps).

This issue was exposed when attempting to use the loopback network interface. I've tried a few different distros, and on all of them the broadcast mask was unset (0.0.0.0). However, on all of them the following route was set (127.255.255.255 being the broadcast address for 127.0.0.1/8) and resulted in broadcasts being transmitted and received properly:

$ ip route get 127.255.255.255 broadcast 127.255.255.255 dev lo table local src 127.0.0.1 uid 1000 cache <local,brd>

marckleinebudde commented 1 year ago

Although the style of the code is fixes, I cannot say anything to the contents of this PR. Maybe @hartkopp can help.

hartkopp commented 1 year ago

Although the style of the code is fixes, I cannot say anything to the contents of this PR. Maybe @hartkopp can help.

Not really! I only show up in that code section due to style fixes. I assume @dschanoeh knows better ...

dschanoeh commented 1 year ago

I'm not sure if I'm the right one to judge since I haven't touched or used this in years but my interpretation is that there is something wrong in those distros - I couldn't find anything regarding a deprecation of SIOCGIFBRDADDR or known limitations. So the original code doesn't seem obviously wrong. Nevertheless, calculating it manually seems like a good workaround and I don't see other problems arising from this.

marckleinebudde commented 1 year ago

Ok - then let's merge this.

faisal-shah commented 1 year ago

Thanks for being responsive!