Thread (51 messages) 51 messages, 11 authors, 2015-03-30

Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets

From: Scott Feldman <hidden>
Date: 2015-03-20 18:03:43

On Fri, Mar 20, 2015 at 9:58 AM,  [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Roopa Prabhu <redacted>

On a Linux bridge with bridge forwarding offloaded to switch ASIC,
there is a need to not re-forward frames that have already been
forwarded in hardware.

Typically these are broadcast or multicast frames forwarded by the
hardware to multiple destination ports including sending a copy of
the packet to the cpu (kernel e.g. an arp broadcast).
The bridge driver will try to forward the packet again, resulting in
two copies of the same packet.

These packets can also come up to the kernel for logging when they hit
a LOG acl rule in hardware. In such cases, you do want the packet
to go through the bridge netfilter hooks. Hence, this patch adds the
required checks just before the packet is being xmited.

v2:
        - Add a new hw_fwded flag in skbuff to indicate that the packet
        is already hardware forwarded. Switch driver will set this flag.
        I have been trying to avoid having this flag in the skb
        and thats why this patch has been in my tree for long. Cant think
        of other better alternatives. Suggestions are welcome. I have put
        this under CONFIG_NET_SWITCHDEV to minimize the impact.

Signed-off-by: Roopa Prabhu <redacted>
Signed-off-by: Wilson Kok <redacted>
---
 include/linux/skbuff.h  |    7 +++++--
 net/bridge/br_forward.c |   11 +++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index bba1330..1973b5c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -560,8 +560,11 @@ struct sk_buff {
                                fclone:2,
                                peeked:1,
                                head_frag:1,
-                               xmit_more:1;
-       /* one bit hole */
+                               xmit_more:1,
+#ifdef CONFIG_NET_SWITCHDEV
+                               hw_fwded:1;
+#endif
+       /* one bit hole if CONFIG_NET_SWITCHDEV not defined */
Did you want this flag not copied in __copy_skb_header()?  Seems those
flags are special cased.  There is room for a bit here:

#ifdef CONFIG_IPV6_NDISC_NODETYPE
        __u8                    ndisc_nodetype:2;
#endif
        __u8                    ipvs_property:1;
        __u8                    inner_protocol_type:1;
        __u8                    remcsum_offload:1;
        /* 3 or 5 bit hole */
         <<<<<<<<
quoted hunk ↗ jump to hunk
        kmemcheck_bitfield_end(flags1);

        /* fields enclosed in headers_start/headers_end are copied
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 3304a54..b60b96e 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -37,6 +37,17 @@ static inline int should_deliver(const struct net_bridge_port *p,

 int br_dev_queue_push_xmit(struct sk_buff *skb)
 {
+
+#ifdef CONFIG_NET_SWITCHDEV
+       /* If skb is already hw forwarded and the port being forwarded
+        * to is a switch port, dont reforward
+        */
+       if (skb->hw_fwded && (skb->dev->features & NETIF_F_HW_SWITCH_OFFLOAD)) {
The  check for skb->dev->features & NETIF_F_HW_SWITCH_OFFLOAD is redundant.
+               kfree_skb(skb);
+               return 0;
+       }
+
+#endif
        if (!is_skb_forwardable(skb->dev, skb)) {
                kfree_skb(skb);
        } else {
--
1.7.10.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help