netdev - Re: [PATCH net-next v2 1/3] dsa: add support for Arrow XRS700x tag trailer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201126140126.GL2075216@lunn.ch>
Date:   Thu, 26 Nov 2020 15:01:26 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     George McCollister <george.mccollister@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller " <davem@...emloft.net>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/3] dsa: add support for Arrow XRS700x tag
 trailer

On Thu, Nov 26, 2020 at 03:50:04PM +0200, Vladimir Oltean wrote:
> On Wed, Nov 25, 2020 at 09:34:29PM +0100, Andrew Lunn wrote:
> > > +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;
> >
> > Something for Vladimir:
> >
> > Could this linearise be moved into the core, depending on the
> > tail_tag?
> 
> Honestly I believe that the skb_linearize is not needed at all.

Humm

I'm assuming this is here in case the frame is in fragments, and the
trailer could be spread over two fragments? If so, you cannot access
the trailer using straight pointers. Linearize should copy it into one
buffer.

For the normal case of a 1500 byte frame, i doubt we have hardware
which uses multiple scatter/gather buffers. But for jumbo frames?

> > > +	if (pskb_trim_rcsum(skb, skb->len - 1))
> > > +		return NULL;
> >
> > And the overhead is also in dsa_devlink_ops, so maybe this can be
> > moved as well?
> 
> Sorry, I don't understand this comment.

I'm meaning, could that also be moved into the core? We seem to have
the needed information to do it in the core.

    Andrew