| Message ID | 20170321222845.8373-1-linus.luessing@c0d3.blue |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Tue, 21 Mar 2017 23:28:45 +0100 Linus Lüssing <linus.luessing@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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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@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(-)