netdev - Re: [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in separate functions.

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: <20130110102525.2ff40c12@nehalam.linuxnetplumber.net>
Date:	Thu, 10 Jan 2013 10:25:25 -0800
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Vlad Yasevich <vyasevic@...hat.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, stephen@...hat.com,
	bridge@...ts.linux-foundation.org, shmulik.ladkani@...il.com,
	mst@...hat.com
Subject: Re: [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in
 separate functions.

On Wed,  9 Jan 2013 12:17:48 -0500
Vlad Yasevich <vyasevic@...hat.com> wrote:

>  
>  /**
> + * vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
> + * @dev: netdevice of the lowerdev/hw nic
> + *
> + * Checks to see if HW and driver report VLAN acceleration correctly.
> + */
> +static inline bool vlan_hw_buggy(const struct net_device *dev)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid))
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + * vlan_vid_add_hw - Add the VLAN vid to the HW filter
> + * @dev: netdevice of the lowerdev/hw nic
> + * @vid: vlan id.
> + *
> + * Inserts the vid into the HW vlan filter table if hw supports it.
> + */
> +static inline int vlan_vid_add_hw(struct net_device *dev,
> +				  unsigned short vid)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +	int err = 0;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    ops->ndo_vlan_rx_add_vid)
> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
> +
> +	return err;
> +}
> +
> +/**
> + * vlan_vid_del_hw - Delete the VLAN vid from the HW filter
> + * @dev: netdevice of the lowerdev/hw nic
> + * @vid: vlan id.
> + *
> + * Delete the vid from the HW vlan filter table if hw supports it.
> + */
> +static inline int vlan_vid_del_hw(struct net_device *dev,
> +				  unsigned short vid)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +	int err = 0;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    ops->ndo_vlan_rx_kill_vid)
> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
> +
> +	return err;
> +}
> +

I would rather not have all these inline's. This isn't performance critical.
Also, the check for buggy devices should be done inside the vlan code,
not repeated in the functions using the add/remove API. When device is
registered the flag and add/kill should be checked, and if the device driver
is buggy it should fail the register_netdevice.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html