On Wed, Jan 13, 2021 at 7:05 PM Vladimir Oltean <olteanv@xxxxxxxxx> wrote:
> > +++ 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?
What Andrew said. I started with tag_trailer.c since it is quite
similar and it seemed wrong to remove the 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.
Removed.
>
> > +#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..
Sorry about this. You and Andrew started discussing it and I guess I
got distracted fixing the other issues. Removed. I'll retest after
making other changes to patches in the series but based on what you've
said it should be fine without it.
>
> > +
> > + 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;
> > +}
