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]
Date:   Wed, 27 Feb 2019 20:09:44 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>, davem@...emloft.net,
        grygorii.strashko@...com
Cc:     linux-omap@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, jiri@...lanox.com,
        ilias.apalodimas@...aro.org
Subject: Re: [PATCH net-next 2/6] net: 8021q: vlan_dev: add vid tag to
 addresses of uc and mc lists



On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> Update vlan mc and uc addresses with VID tag while propagating
> addresses to lower devices, do this only if address is not synced.
> It allows at end driver level to distinguish addresses belonging
> to vlan devices.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
> ---

[snip]

>  
> +u16 vlan_dev_get_addr_vid(struct net_device *dev, const u8 *addr)

Having some kernel doc comment here would also be nice.

> +{
> +	u16 vid = 0;
> +
> +	if (dev->vid_len != NET_8021Q_VID_TSIZE)
> +		return vid;
> +
> +	vid = addr[dev->addr_len];
> +	vid |= (addr[dev->addr_len + 1] & 0xf) << 8;

This uses knowledge of the maximum VLAN ID is 4095, which is fine, might
be a good idea to add a check on VID not exceeding the maximum VLAN ID
number instead of doing a silent truncation?

[snip]

> +static void vlan_dev_align_addr_vid(struct net_device *vlan_dev)
> +{
> +	struct net_device *real_dev = vlan_dev_real_dev(vlan_dev);
> +	struct netdev_hw_addr *ha;
> +
> +	if (!real_dev->vid_len)
> +		return;

Should not this check be moved to dev_{mc,uc}_sync? It does not seem to
me like this would scale really well across different stacked devices
(VLAN, bond, macvlan) as well as underlying drivers (cpsw, dsa, etc.).
Or maybe the check should be if vlan_dev->vid_len > real_dev->vid_len ->
error, right?
-- 
Florian

Powered by blists - more mailing lists