Linus Lüssing
2017-Mar-19 16:55 UTC
[Bridge] [PATCH net] bridge: ebtables: fix reception of frames DNAT-ed to bridge device
On Fri, Mar 17, 2017 at 02:10:44PM +0100, Pablo Neira Ayuso wrote:> Wait. > > May this break local multicast listener that are bound to the bridge > interface? Assuming the bridge interface got an IP address, and that > there is local multicast listener. > > Missing anything here?Hm, for multicast packets usually the code path a few lines later in br_handle_frame_finish() should be taken instead. But you might be right for IP multicast packets with a unicast MAC destination (due to whatever reason, for instance via DNAT'ing again). Will check that - thanks!
Linus Lüssing
2017-Mar-21 00:09 UTC
[Bridge] [PATCH net] bridge: ebtables: fix reception of frames DNAT-ed to bridge device
On Sun, Mar 19, 2017 at 05:55:06PM +0100, Linus L?ssing wrote:> On Fri, Mar 17, 2017 at 02:10:44PM +0100, Pablo Neira Ayuso wrote: > > Wait. > > > > May this break local multicast listener that are bound to the bridge > > interface? Assuming the bridge interface got an IP address, and that > > there is local multicast listener. > > > > Missing anything here? > > Hm, for multicast packets usually the code path a few lines > later in br_handle_frame_finish() should be taken instead. > > But you might be right for IP multicast packets with a unicast MAC > destination (due to whatever reason, for instance via DNAT'ing > again). > > Will check that - thanks!Ok, I tested DNAT'ing an IP multicast packet to the unicast MAC address of the bridge interface. Both ping-ing to an IPv4 and IPv6 multicast listener on br0 worked and was replied to fine, both with or without changing skb->pkt_type from PACKET_MULTICAST to PACKET_HOST. ("$ ping 224.1.0.123" and "$ ping6 ff02::1:ff40:707c%in0" from a network namespace, tied into the bridge via veth) Also, a DNAT'ed PACKET_BROADCAST worked, with or without changing it to PACKET_HOST. I also checked via tcpdump that the destination MAC was changed successfully. So, so far I wasn't able to find any bugs with the current patch. But I think I like the idea of leaving the skb->pkt_type unaltered for PACKET_MULTICAST and PACKET_BROADCAST, seems cleaner. I'd just add an "if (skb->pkt_type == PACKET_OTHERHOST)" check then and resend a PATCH v2.