netdev - Re: [PATCH V2 net-next 1/2] liquidio: switchdev support for LiquidIO NIC

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: <20171027164505.4cc0df92@cakuba.netronome.com>
Date:   Fri, 27 Oct 2017 16:45:05 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Felix Manlunas <felix.manlunas@...ium.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        raghu.vatsavayi@...ium.com, derek.chickles@...ium.com,
        satananda.burla@...ium.com, vijaya.guvva@...ium.com
Subject: Re: [PATCH V2 net-next 1/2] liquidio: switchdev support for
 LiquidIO NIC

On Fri, 27 Oct 2017 12:09:17 -0700, Felix Manlunas wrote:
> From: Vijaya Mohan Guvva <vijaya.guvva@...ium.com>
> 
> Enable switchdev for SRIOV capable LiquidIO NIC. It registers
> a representor netdev (with switchdev_ops) for each SRIOV VF created.
> It also has changes to send representor interface configurations like
> admin state and MTU to LiquidIO firmware and to retrieve HW counted
> VF stats for VF representor.
> 
> Signed-off-by: Vijaya Mohan Guvva <vijaya.guvva@...ium.com>
> Signed-off-by: Satanand Burla <satananda.burla@...ium.com>
> Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@...ium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@...ium.com>

Thanks for making the changes!  One more thing caught my eye this time
around.

> +static int
> +lio_pf_switchdev_attr_get(struct net_device *dev, struct switchdev_attr *attr)
> +{
> +	struct lio *lio = GET_LIO(dev);
> +	struct octeon_device *oct = lio->oct_dev;
> +	char phy_switch_id[] = {0x17, 0x7d, 0x97, 0x02, 0x00};
> +	int len = sizeof(phy_switch_id);
> +
> +	switch (attr->id) {
> +	case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
> +		phy_switch_id[len - 1] = oct->pf_num;
> +		attr->u.ppid.id_len = len;
> +		memcpy(attr->u.ppid.id, phy_switch_id, len);
> +		break;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}

> +static int
> +lio_vf_rep_attr_get(struct net_device *dev, struct switchdev_attr *attr)
> +{
> +	char phy_switch_id[] = {0x17, 0x7d, 0x97, 0x02, 0x00};
> +	struct lio_vf_rep_desc *vf_rep = netdev_priv(dev);
> +	struct octeon_device *oct = vf_rep->oct;
> +	int len = sizeof(phy_switch_id);
> +
> +	switch (attr->id) {
> +	case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
> +		phy_switch_id[len - 1] = oct->pf_num;
> +		attr->u.ppid.id_len = len;
> +		memcpy(attr->u.ppid.id, phy_switch_id, len);
> +		break;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}

SWITCHDEV_ATTR_ID_PORT_PARENT_ID is generally used to determine whether
two ports belong to the same switch.  Do you not want to support rules
which redirect packets from one PF (and it's VFs) to the other?

Also if you plug two cards into one system, the above will make you
offload redirect between the cards.  Is that what you want?  Commonly
the MAC address of the first port or serial number would be used here
to make sure IDs are unique for each device.