Subject: Re: [Intel-wired-lan] [PATCH v6 2/3] ixgbe: Add new ndo to trust VF

From: Alexander Duyck <[email-masked]>
Date: Wed, 17 Jun 2015 09:22:09 -0700

On 06/17/2015 04:43 AM, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <[email-masked]>
>
> Implements the new netdev op to trust VF in ixgbe.
>
> The administrator can turn on and off VF trusted by ip command which
> supports trust message.
>   # ip link set dev eth0 vf 1 trust on
> or
>   # ip link set dev eth0 vf 1 trust off
>
> Send a ping to reset VF on changing the status of trusting.
> VF driver will reconfigure its features on reset.
>
> Signed-off-by: Hiroshi Shimamoto <[email-masked]>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe.h       |  1 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  1 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 45 ++++++++++++++++++++++----
>   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |  1 +
>   4 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index 8830c0f..7f76c12 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -152,6 +152,7 @@ struct vf_data_storage {
>   	u16 vlan_count;
>   	u8 spoofchk_enabled;
>   	bool rss_query_enabled;
> +	u8 trusted;
>   	unsigned int vf_api;
>   };
>

At some point these boolean values should be converted to bit flags to 
save space.  At this point it looks like you have something like 5 
boolean values now in this structure.  If you converted them to a u8 
field of bit flags you could save about 4 bytes per instance.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 5f1b06a..376b49b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -8349,6 +8349,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
>   	.ndo_set_vf_rate	= ixgbe_ndo_set_vf_bw,
>   	.ndo_set_vf_spoofchk	= ixgbe_ndo_set_vf_spoofchk,
>   	.ndo_set_vf_rss_query_en = ixgbe_ndo_set_vf_rss_query_en,
> +	.ndo_set_vf_trust	= ixgbe_ndo_set_vf_trust,
>   	.ndo_get_vf_config	= ixgbe_ndo_get_vf_config,
>   	.ndo_get_stats64	= ixgbe_get_stats64,
>   #ifdef CONFIG_IXGBE_DCB
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 1d17b58..826f88e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -116,6 +116,9 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
>   			 * we want to disable the querying by default.
>   			 */
>   			adapter->vfinfo[i].rss_query_enabled = 0;
> +
> +			/* Untrust all VFs */
> +			adapter->vfinfo[i].trusted = false;
>   		}
>
>   		return 0;
> @@ -1124,18 +1127,23 @@ void ixgbe_disable_tx_rx(struct ixgbe_adapter *adapter)
>   	IXGBE_WRITE_REG(hw, IXGBE_VFRE(1), 0);
>   }
>
> -void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter)
> +static inline void ixgbe_ping_vf(struct ixgbe_adapter *adapter, int vf)
>   {
>   	struct ixgbe_hw *hw = &adapter->hw;
>   	u32 ping;
> +
> +	ping = IXGBE_PF_CONTROL_MSG;
> +	if (adapter->vfinfo[vf].clear_to_send)
> +		ping |= IXGBE_VT_MSGTYPE_CTS;
> +	ixgbe_write_mbx(hw, &ping, 1, vf);
> +}
> +
> +void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter)
> +{
>   	int i;
>
> -	for (i = 0 ; i < adapter->num_vfs; i++) {
> -		ping = IXGBE_PF_CONTROL_MSG;
> -		if (adapter->vfinfo[i].clear_to_send)
> -			ping |= IXGBE_VT_MSGTYPE_CTS;
> -		ixgbe_write_mbx(hw, &ping, 1, i);
> -	}
> +	for (i = 0 ; i < adapter->num_vfs; i++)
> +		ixgbe_ping_vf(adapter, i);
>   }
>

This ping_all_vfs/ping_vf change doesn't really have anything to do with 
the ndo_trust part and should probably be a separate patch.

>   int ixgbe_ndo_set_vf_mac(struct net_device *netdev, int vf, u8 *mac)
> @@ -1416,6 +1424,28 @@ int ixgbe_ndo_set_vf_rss_query_en(struct net_device *netdev, int vf,
>   	return 0;
>   }
>
> +int ixgbe_ndo_set_vf_trust(struct net_device *netdev, int vf, bool setting)
> +{
> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> +
> +	if (vf >= adapter->num_vfs)
> +		return -EINVAL;
> +
> +	/* nothing to do */
> +	if (adapter->vfinfo[vf].trusted == setting)
> +		return 0;
> +
> +	adapter->vfinfo[vf].trusted = setting;
> +
> +	/* reset VF to reconfigure features */
> +	adapter->vfinfo[vf].clear_to_send = false;
> +	ixgbe_ping_vf(adapter, vf);
> +
> +	e_info(drv, "VF %u is %strusted\n", vf, setting ? "" : "not ");
> +
> +	return 0;
> +}
> +
>   int ixgbe_ndo_get_vf_config(struct net_device *netdev,
>   			    int vf, struct ifla_vf_info *ivi)
>   {
> @@ -1430,5 +1460,6 @@ int ixgbe_ndo_get_vf_config(struct net_device *netdev,
>   	ivi->qos = adapter->vfinfo[vf].pf_qos;
>   	ivi->spoofchk = adapter->vfinfo[vf].spoofchk_enabled;
>   	ivi->rss_query_en = adapter->vfinfo[vf].rss_query_enabled;
> +	ivi->trusted = adapter->vfinfo[vf].trusted;
>   	return 0;
>   }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
> index 2c197e6..dad9257 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
> @@ -49,6 +49,7 @@ int ixgbe_ndo_set_vf_bw(struct net_device *netdev, int vf, int min_tx_rate,
>   int ixgbe_ndo_set_vf_spoofchk(struct net_device *netdev, int vf, bool setting);
>   int ixgbe_ndo_set_vf_rss_query_en(struct net_device *netdev, int vf,
>   				  bool setting);
> +int ixgbe_ndo_set_vf_trust(struct net_device *netdev, int vf, bool setting);
>   int ixgbe_ndo_get_vf_config(struct net_device *netdev,
>   			    int vf, struct ifla_vf_info *ivi);
>   void ixgbe_check_vf_rate_limit(struct ixgbe_adapter *adapter);
>