netdev - RE: [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail tagging

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: <9235D6609DB808459E95D78E17F2E43D40A5C086@CHN-SV-EXMX02.mchp-main.com>
Date:   Sat, 6 May 2017 00:35:04 +0000
From:   <Woojung.Huh@...rochip.com>
To:     <f.fainelli@...il.com>, <andrew@...n.ch>,
        <vivien.didelot@...oirfairelinux.com>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH net-next 1/5] dsa: add support for Microchip KSZ tail
 tagging

> > +	padlen = 0;
> > +	if (skb->len < 60)
> > +		padlen = 60 - skb->len;
> 
> Can you use ETH_ZLEN instead of 60 such that it is clear this is padding
> to a minimum packet size (minus FCS)?
> 
> > +
> > +	nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 2,
> GFP_ATOMIC);
> 
> Can you also define the "2" at the beginning of the file as being e.g:
> TAG_KSZ_LEN?
Got it. Will update.

> > +	ds = dst->cpu_switch;
> > +
> > +	if (skb_linearize(skb))
> > +		return NULL;
> 
> Is that really necessary?
I don't think so. Will check it.

> > +
> > +	tag = skb_tail_pointer(skb) - 1;
> > +
> > +	source_port = tag[0] & 7;
> > +	if (source_port >= ds->num_ports || !ds-
> >ports[source_port].netdev)
> > +		return NULL;
> > +
> > +	pskb_trim_rcsum(skb, skb->len - 1);
> 
> Humm, so we are still keeping tag[1] at the end of the frame?
It tags 2 bytes for ingress (ksz_xmit) and 1 byte for egress (ksz_rcv).
Put comment at the begin of net/dsa/tag_ksz.c.

Thanks.
Woojung