Message ID | 1418052460-30691-1-git-send-email-jouni@codeaurora.org |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, 8 Dec 2014 17:27:40 +0200 Jouni Malinen <jouni@codeaurora.org> wrote: > From: Kyeyoon Park <kyeyoonp@codeaurora.org> > > Because dropping broadcast packets for IEEE 802.11 Proxy ARP is more > selective than previously thought, it is better to remove the direct > dropping logic in the bridge code in favor of using the netfilter > infrastructure to provide more control on which frames get dropped. This > code was added in commit 958501163ddd ("bridge: Add support for IEEE > 802.11 Proxy ARP"). > > Signed-off-by: Kyeyoon Park <kyeyoonp@codeaurora.org> > Signed-off-by: Jouni Malinen <jouni@codeaurora.org> > --- > net/bridge/br_forward.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > index f96933a..8a025a7 100644 > --- a/net/bridge/br_forward.c > +++ b/net/bridge/br_forward.c > @@ -185,10 +185,6 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb, > if (unicast && !(p->flags & BR_FLOOD)) > continue; > > - /* Do not flood to ports that enable proxy ARP */ > - if (p->flags & BR_PROXYARP) > - continue; > - > prev = maybe_deliver(prev, p, skb, __packet_hook); > if (IS_ERR(prev)) > goto out; Aren't you at risk of duplicate ARP responses in some cases. You can't assume user will run netfilter. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 09, 2014 at 02:21:58PM -0800, Stephen Hemminger wrote: > On Mon, 8 Dec 2014 17:27:40 +0200 > Jouni Malinen <jouni@codeaurora.org> wrote: > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > > @@ -185,10 +185,6 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb, > > if (unicast && !(p->flags & BR_FLOOD)) > > continue; > > > > - /* Do not flood to ports that enable proxy ARP */ > > - if (p->flags & BR_PROXYARP) > > - continue; > > - > > prev = maybe_deliver(prev, p, skb, __packet_hook); > Aren't you at risk of duplicate ARP responses in some cases. > You can't assume user will run netfilter. This is only for the case where BR_PROXYARP has been enabled by the user, but yes, it would be convenient to handle cases better without requiring netfilter and skip flooding to BR_PROXYARP port more selectively here. Would there be some convenient means for br_do_proxy_arp() to mark the skb that it has replied to in br_input.c and then use that here in br_forward.c to not flood an ARP request that has already been replied to? Or should this simply skip flooding of all ARP packets with something like following? if (unicast && !(p->flags & BR_FLOOD)) continue; - /* Do not flood to ports that enable proxy ARP */ - if (p->flags & BR_PROXYARP) + /* Do not flood ARP to ports that enable proxy ARP */ + if (p->flags & BR_PROXYARP && + skb->protocol == htons(ETH_P_ARP)) continue; prev = maybe_deliver(prev, p, skb, __packet_hook);
On Tue, Dec 09, 2014 at 02:21:58PM -0800, Stephen Hemminger wrote: > On Mon, 8 Dec 2014 17:27:40 +0200 > > From: Kyeyoon Park <kyeyoonp@codeaurora.org> > > Because dropping broadcast packets for IEEE 802.11 Proxy ARP is more > > selective than previously thought, it is better to remove the direct > > dropping logic in the bridge code in favor of using the netfilter > > infrastructure to provide more control on which frames get dropped. This > > code was added in commit 958501163ddd ("bridge: Add support for IEEE > > 802.11 Proxy ARP"). > Aren't you at risk of duplicate ARP responses in some cases. > You can't assume user will run netfilter. The 'bridge: Selectively prevent bridge port flooding for proxy ARP' patch that I posted earlier today addresses this by keeping record of whether the proxyarp functionality has replied to a packet and skipping flooding conditionally on that. In other words, this 'bridge: Remove BR_PROXYARP flooding check code' can be dropped.
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c index f96933a..8a025a7 100644 --- a/net/bridge/br_forward.c +++ b/net/bridge/br_forward.c @@ -185,10 +185,6 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb, if (unicast && !(p->flags & BR_FLOOD)) continue; - /* Do not flood to ports that enable proxy ARP */ - if (p->flags & BR_PROXYARP) - continue; - prev = maybe_deliver(prev, p, skb, __packet_hook); if (IS_ERR(prev)) goto out;