Linux Ethernet Bridging - Mar 2017 - [Bridge] [PATCH v2] bridge: ebtables: fix reception of frames DNAT-ed to bridge device

When trying to redirect bridged frames to the bridge device itself
via the ebtables nat-prerouting chain and the dnat target then this
currently fails:

The ethernet destination of the frame is dnat'ed to the MAC address of
the bridge itself just fine and the correctly altered frame can even
be captured via a tcpdump on br0 (with or without promisc mode).

However, the IP code drops it in the beginning of ip_input.c/ip_rcv()
as the dnat target did not update the skb->pkt_type. If after
dnat'ing the packet is now destined to us then the skb->pkt_type
needs to be updated from PACKET_OTHERHOST to PACKET_HOST, too.

Signed-off-by: Linus L?ssing <linus.luessing at c0d3.blue>

---
Changelog v2:
* refrain from altering pkt_type for multicast packets
  with a unicast destination MAC
---
 net/bridge/br_input.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 013f2290b..fd7bc4c 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -198,8 +198,13 @@ int br_handle_frame_finish(struct net *net, struct sock
*sk, struct sk_buff *skb
 	if (dst) {
 		unsigned long now = jiffies;
 
-		if (dst->is_local)
+		if (dst->is_local) {
+			/* fix up potential DNAT inconsistencies */
+			if (skb->pkt_type == PACKET_OTHERHOST)
+				skb->pkt_type = PACKET_HOST;
+
 			return br_pass_frame_up(skb);
+		}
 
 		if (now != dst->used)
 			dst->used = now;
-- 
2.1.4

On Tue, 21 Mar 2017 23:28:45 +0100
Linus L?ssing <linus.luessing at c0d3.blue> wrote:
> However, the IP code drops it in the beginning of ip_input.c/ip_rcv()
> as the dnat target did not update the skb->pkt_type. If after
> dnat'ing the packet is now destined to us then the skb->pkt_type
> needs to be updated from PACKET_OTHERHOST to PACKET_HOST, too.
Why not fix DNAT netfilter module rather than hacking bridge code here.

On Tue, Mar 21, 2017 at 04:32:45PM -0700, Stephen Hemminger
wrote:
> On Tue, 21 Mar 2017 23:28:45 +0100
> Linus L?ssing <linus.luessing at c0d3.blue> wrote:
> 
> > However, the IP code drops it in the beginning of ip_input.c/ip_rcv()
> > as the dnat target did not update the skb->pkt_type. If after
> > dnat'ing the packet is now destined to us then the
skb->pkt_type
> > needs to be updated from PACKET_OTHERHOST to PACKET_HOST, too.
> 
> Why not fix DNAT netfilter module rather than hacking bridge code here.
Sorry for the late response. Wanted to do some more testing before
replying.

My assumptions regarding macvlan were wrong:

A) The code my patch adds is not touched in the case of a macvlan
device on top of a bridge - with macvlan there seems to be no FDB entry
for the MAC address of the macvlan device at all, actually, resulting
in the flooding of a frame intended for that device (is this, ehm,
known/intended?).

B) ip_rcv() does not drop for a macvlan device, because macvlan
unconditionally sets skb->pkt_type to PACKET_HOST in macvlan_handle_frame().

(And I guess, maybe I shouldn't actually care that much about
macvlans on top of a bridge, as a veth-pair is much cleaner for that?)


Regarding netdev::dev_addrs, if I understand the kernel code correctly, it
is only, potentially populated with more than netdev::dev_addr for two
drivers, via dev_addr_add(): bnx2x and ixgbe. So for a bridge device,
there should never be more than one item in netdev::dev_addrs
(= netdev::dev_addr), correct?


So, currently I'm happy with moving the fix into the ebtables dnat
code and just checking against the netdev::dev_addr of the bridge
device now. (if anyone has any suggestions regarding upper devices
I should test with that, then please let me know)

(Sorry, Pablo, for me pressing against your earlier suggestion to
put the fix in the ebtables dnat instead of bridge code. :( )

Regards, Linus