On Sat, Jul 21, 2018 at 09:13:56PM +0200, Hauke Mehrtens wrote:

This handles the tag added by the PMAC on the VRX200 SoC line.

The GSWIP uses internally a GSWIP special tag which is located after the
Ethernet header. The PMAC which connects the GSWIP to the CPU converts
this special tag used by the GSWIP into the PMAC special tag which is
added in front of the Ethernet header.

This was tested with GSWIP 2.0 found in the VRX200 SoCs, other GSWIP
versions use slightly different PMAC special tags

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

Hi Hauke

This looks good. A new minor nitpicks below.

+#include <linux/bitops.h>
+#include <linux/etherdevice.h>
+#include <linux/skbuff.h>
+#include <net/dsa.h>
+
+#include "dsa_priv.h"
+
+
+#define GSWIP_TX_HEADER_LEN		4

Single newline is sufficient.

+/* Byte 3 */
+#define GSWIP_TX_CRCGEN_DIS		BIT(23)

BIT(23) in a byte is a bit odd.

+#define GSWIP_TX_SLPID_SHIFT		0	/* source port ID */
+#define  GSWIP_TX_SLPID_CPU		2
+#define  GSWIP_TX_SLPID_APP1		3
+#define  GSWIP_TX_SLPID_APP2		4
+#define  GSWIP_TX_SLPID_APP3		5
+#define  GSWIP_TX_SLPID_APP4		6
+#define  GSWIP_TX_SLPID_APP5		7
+
+
+#define GSWIP_RX_HEADER_LEN	8

Single newline is sufficient. Please fix them all, if there are more
of them.

+
+/* special tag in RX path header */
+/* Byte 7 */
+#define GSWIP_RX_SPPID_SHIFT		4
+#define GSWIP_RX_SPPID_MASK		GENMASK(6, 4)
+
+static struct sk_buff *gswip_tag_rcv(struct sk_buff *skb,
+				     struct net_device *dev,
+				     struct packet_type *pt)
+{
+	int port;
+	u8 *gswip_tag;
+
+	if (unlikely(!pskb_may_pull(skb, GSWIP_RX_HEADER_LEN)))
+		return NULL;
+
+	gswip_tag = ((u8 *)skb->data) - ETH_HLEN;

The cast should not be needed, data already is an unsigned char.

+	skb_pull_rcsum(skb, GSWIP_RX_HEADER_LEN);
+
+	/* Get source port information */
+	port = (gswip_tag[7] & GSWIP_RX_SPPID_MASK) >> GSWIP_RX_SPPID_SHIFT;
+	skb->dev = dsa_master_find_slave(dev, 0, port);
+	if (!skb->dev)
+		return NULL;
+
+	return skb;
+}

  Andrew