Re: [PATCH net-next v4 1/3] dsa: add support for Arrow XRS700x tag trailer
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-01-14 01:08:33
Also in:
netdev
On Wed, Jan 13, 2021 at 08:59:20AM -0600, George McCollister wrote:
Add support for Arrow SpeedChips XRS700x single byte tag trailer. This is modeled on tag_trailer.c which works in a similar way. Signed-off-by: George McCollister <george.mccollister@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> ---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com> A few comments below.
quoted hunk ↗ jump to hunk
diff --git a/net/dsa/tag_xrs700x.c b/net/dsa/tag_xrs700x.c new file mode 100644 index 000000000000..4ee7c260a8a9 --- /dev/null +++ b/net/dsa/tag_xrs700x.c@@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * XRS700x tag format handling + * Copyright (c) 2008-2009 Marvell Semiconductor
Why does Marvell get copyright?
+ * Copyright (c) 2020 NovaTech LLC + */ + +#include <linux/etherdevice.h> +#include <linux/list.h> +#include <linux/slab.h>
These 3 includes are not needed. You can probably remove them later though, if there is no other reason to resend.
+#include <linux/bitops.h>
+
+#include "dsa_priv.h"
+
+static struct sk_buff *xrs700x_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ u8 *trailer;
+
+ trailer = skb_put(skb, 1);
+ trailer[0] = BIT(dp->index);
+
+ return skb;
+}
+
+static struct sk_buff *xrs700x_rcv(struct sk_buff *skb, struct net_device *dev,
+ struct packet_type *pt)
+{
+ int source_port;
+ u8 *trailer;
+
+ if (skb_linearize(skb))
+ return NULL;We've been through this, there should be no reason to linearize an skb for a one-byte tail tag..
+ + trailer = skb_tail_pointer(skb) - 1; + + source_port = ffs((int)trailer[0]) - 1; + + if (source_port < 0) + return NULL; + + skb->dev = dsa_master_find_slave(dev, 0, source_port); + if (!skb->dev) + return NULL; + + if (pskb_trim_rcsum(skb, skb->len - 1)) + return NULL; + + /* Frame is forwarded by hardware, don't forward in software. */ + skb->offload_fwd_mark = 1; + + return skb; +}