netdev - Re: [net-next 03/15] i40evf: support ethtool RSS options

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: <1398774929.3881.7.camel@deadeye.wl.decadent.org.uk>
Date:	Tue, 29 Apr 2014 13:35:29 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Cc:	davem@...emloft.net, Mitch A Williams <mitch.a.williams@...el.com>,
	netdev@...r.kernel.org, gospo@...hat.com, sassmann@...hat.com
Subject: Re: [net-next 03/15] i40evf: support ethtool RSS options

On Mon, 2014-04-28 at 06:52 -0700, Jeff Kirsher wrote:
[...]
> +/**
> + * i40evf_set_rss_hash_opt - Enable/Disable flow types for RSS hash
> + * @adapter: board private structure
> + * @cmd: ethtool rxnfc command
> + *
> + * Returns Success if the flow input set is supported.
> + **/
> +static int i40evf_set_rss_hash_opt(struct i40evf_adapter *adapter,
> +				   struct ethtool_rxnfc *nfc)
> +{
[...]
> +	case AH_ESP_V4_FLOW:
> +	case AH_V4_FLOW:
> +	case ESP_V4_FLOW:
> +	case SCTP_V4_FLOW:
> +		if ((nfc->data & RXH_L4_B_0_1) ||
> +		    (nfc->data & RXH_L4_B_2_3))
> +			return -EINVAL;
> +		hena |= ((u64)1 << I40E_FILTER_PCTYPE_NONF_IPV4_OTHER);
> +		break;
> +	case AH_ESP_V6_FLOW:
> +	case AH_V6_FLOW:
> +	case ESP_V6_FLOW:
> +	case SCTP_V6_FLOW:
> +		if ((nfc->data & RXH_L4_B_0_1) ||
> +		    (nfc->data & RXH_L4_B_2_3))
> +			return -EINVAL;
> +		hena |= ((u64)1 << I40E_FILTER_PCTYPE_NONF_IPV6_OTHER);
> +		break;

I'm not sure why these cases operate on 'hena' as
I40E_FILTER_PCTYPE_NONF_IPV{4,6}_OTHER are presumably always set in the
register.

> +	case IPV4_FLOW:
> +		hena |= ((u64)1 << I40E_FILTER_PCTYPE_NONF_IPV4_OTHER) |
> +			((u64)1 << I40E_FILTER_PCTYPE_FRAG_IPV4);
> +		break;
> +	case IPV6_FLOW:
> +		hena |= ((u64)1 << I40E_FILTER_PCTYPE_NONF_IPV6_OTHER) |
> +			((u64)1 << I40E_FILTER_PCTYPE_FRAG_IPV6);
> +		break;

These cases aren't validating that the user didn't ask for ports to be
included in the hash.  They also interact oddly with UDP flow hashing
configuration.  It seems to me that the I40E_FILTER_PCTYPE_FRAG_IPV{4,6}
bits ought to be controllable through one flow type only.

[...]
> +static int i40evf_get_rxfh_indir(struct net_device *netdev, u32 *indir)
> +{
> +	struct i40evf_adapter *adapter = netdev_priv(netdev);
> +	struct i40e_hw *hw = &adapter->hw;
> +	u32 hlut_val;
> +	int i, j;
> +
> +	for (i = 0, j = 0; i < I40E_VFQF_HLUT_MAX_INDEX; i++) {
[...]

Off by 1?

Ben.

-- 
Ben Hutchings
Q.  Which is the greater problem in the world today, ignorance or apathy?
A.  I don't know and I couldn't care less.

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)