netdev - Re: [PATCH v2 net-next 3/7] net: dsa: Add Lantiq / Intel GSWIP tag support

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 3 Sep 2018 11:52:44 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Hauke Mehrtens <hauke@...ke-m.de>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, john@...ozen.org,
        linux-mips@...ux-mips.org, dev@...sin.me, hauke.mehrtens@...el.com,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/7] net: dsa: Add Lantiq / Intel GSWIP tag
 support



On 9/1/2018 5:03 AM, 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.1 found in the VRX200 SoCs, other GSWIP
> versions use slightly different PMAC special tags.
> 
> Signed-off-by: Hauke Mehrtens <hauke@...ke-m.de>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>

Just one suggestion below, if you need to resubmit this:

[snip]

> +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 = skb->data - ETH_HLEN;
> +	skb_pull_rcsum(skb, GSWIP_RX_HEADER_LEN);

I would be moving this after the port lookup was successful, that way if 
you are discarding a frame, you can do this as quickly as possible, this 
should not have a functional impact since you return a skb with the 
checksum updated past the return of that function.
-- 
Florian