[PATCH 0/2] i40e/i40evf: fix override of administratively set MAC address in trusted mode

[PATCH 0/2] i40e/i40evf: fix override of administratively set MAC address in trusted mode

[Stefan Assmann]

In trusted mode VFs should be able to override the administratively set MAC
address. This is currently not possible as i40evf returns -EPERM if an
administratively set MAC is present, for MAC change requests.

First patch renames vf_offload_flags to vf_cap_flags to make it more generic.
Second patch then adds a flag to vf_cap_flags to give the VF information on
whether it runs in trusted mode and makes use of the flag in i40evf.

Patches are against jkirshers dev-queue branch.

Stefan Assmann (2):
  i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct
    virtchnl_vf_resource
  i40e/i40evf: add virtchnl flag to indicate trusted VF

 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 25 ++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_common.c    |  2 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         | 10 ++++-----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 15 +++++++------
 include/linux/avf/virtchnl.h                       |  5 +++--
 5 files changed, 31 insertions(+), 26 deletions(-)

-- 
2.9.4

[Intel-wired-lan] [PATCH 0/2] i40e/i40evf: fix override of administratively set MAC address in trusted mode

[Stefan Assmann]

In trusted mode VFs should be able to override the administratively set MAC
address. This is currently not possible as i40evf returns -EPERM if an
administratively set MAC is present, for MAC change requests.

First patch renames vf_offload_flags to vf_cap_flags to make it more generic.
Second patch then adds a flag to vf_cap_flags to give the VF information on
whether it runs in trusted mode and makes use of the flag in i40evf.

Patches are against jkirshers dev-queue branch.

Stefan Assmann (2):
  i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct
    virtchnl_vf_resource
  i40e/i40evf: add virtchnl flag to indicate trusted VF

 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 25 ++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_common.c    |  2 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         | 10 ++++-----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 15 +++++++------
 include/linux/avf/virtchnl.h                       |  5 +++--
 5 files changed, 31 insertions(+), 26 deletions(-)

-- 
2.9.4

[PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Stefan Assmann]

The current name of vf_offload_flags indicates that the bitmap is
limited to offload related features. Make this more generic by renaming
it to vf_cap_flags, which allows for other capabilities besides
offloading to be added.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 22 +++++++++++-----------
 drivers/net/ethernet/intel/i40evf/i40e_common.c    |  2 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         | 10 +++++-----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 12 ++++++------
 include/linux/avf/virtchnl.h                       |  4 ++--
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 3ef67dc..057c77b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1528,39 +1528,39 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 				  VIRTCHNL_VF_OFFLOAD_RSS_REG |
 				  VIRTCHNL_VF_OFFLOAD_VLAN;
 
-	vfres->vf_offload_flags = VIRTCHNL_VF_OFFLOAD_L2;
+	vfres->vf_cap_flags = VIRTCHNL_VF_OFFLOAD_L2;
 	vsi = pf->vsi[vf->lan_vsi_idx];
 	if (!vsi->info.pvid)
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_VLAN;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_VLAN;
 
 	if (i40e_vf_client_capable(pf, vf->vf_id) &&
 	    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_IWARP)) {
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_IWARP;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_IWARP;
 		set_bit(I40E_VF_STATE_IWARPENA, &vf->vf_states);
 	}
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_PF;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_PF;
 	} else {
 		if ((pf->hw_features & I40E_HW_RSS_AQ_CAPABLE) &&
 		    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_AQ))
-			vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_AQ;
+			vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_AQ;
 		else
-			vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_REG;
+			vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_REG;
 	}
 
 	if (pf->hw_features & I40E_HW_MULTIPLE_TCP_UDP_RSS_PCTYPE) {
 		if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2)
-			vfres->vf_offload_flags |=
+			vfres->vf_cap_flags |=
 				VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2;
 	}
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_ENCAP)
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP;
 
 	if ((pf->hw_features & I40E_HW_OUTER_UDP_CSUM_CAPABLE) &&
 	    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM))
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM;
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RX_POLLING) {
 		if (pf->flags & I40E_FLAG_MFP_ENABLED) {
@@ -1570,12 +1570,12 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 			aq_ret = I40E_ERR_PARAM;
 			goto err;
 		}
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RX_POLLING;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RX_POLLING;
 	}
 
 	if (pf->hw_features & I40E_HW_WB_ON_ITR_CAPABLE) {
 		if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
-			vfres->vf_offload_flags |=
+			vfres->vf_cap_flags |=
 					VIRTCHNL_VF_OFFLOAD_WB_ON_ITR;
 	}
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index 1dd1938..d69c2e4 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -1104,7 +1104,7 @@ void i40e_vf_parse_hw_config(struct i40e_hw *hw,
 	hw->dev_caps.num_rx_qp = msg->num_queue_pairs;
 	hw->dev_caps.num_tx_qp = msg->num_queue_pairs;
 	hw->dev_caps.num_msix_vectors_vf = msg->max_vectors;
-	hw->dev_caps.dcb = msg->vf_offload_flags &
+	hw->dev_caps.dcb = msg->vf_cap_flags &
 			   VIRTCHNL_VF_OFFLOAD_L2;
 	hw->dev_caps.fcoe = 0;
 	for (i = 0; i < msg->num_vsis; i++) {
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index c89767e..d85e446 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -277,19 +277,19 @@ struct i40evf_adapter {
 	enum virtchnl_link_speed link_speed;
 	enum virtchnl_ops current_op;
 #define CLIENT_ALLOWED(_a) ((_a)->vf_res ? \
-			    (_a)->vf_res->vf_offload_flags & \
+			    (_a)->vf_res->vf_cap_flags & \
 				VIRTCHNL_VF_OFFLOAD_IWARP : \
 			    0)
 #define CLIENT_ENABLED(_a) ((_a)->cinst)
 /* RSS by the PF should be preferred over RSS via other methods. */
-#define RSS_PF(_a) ((_a)->vf_res->vf_offload_flags & \
+#define RSS_PF(_a) ((_a)->vf_res->vf_cap_flags & \
 		    VIRTCHNL_VF_OFFLOAD_RSS_PF)
-#define RSS_AQ(_a) ((_a)->vf_res->vf_offload_flags & \
+#define RSS_AQ(_a) ((_a)->vf_res->vf_cap_flags & \
 		    VIRTCHNL_VF_OFFLOAD_RSS_AQ)
-#define RSS_REG(_a) (!((_a)->vf_res->vf_offload_flags & \
+#define RSS_REG(_a) (!((_a)->vf_res->vf_cap_flags & \
 		       (VIRTCHNL_VF_OFFLOAD_RSS_AQ | \
 			VIRTCHNL_VF_OFFLOAD_RSS_PF)))
-#define VLAN_ALLOWED(_a) ((_a)->vf_res->vf_offload_flags & \
+#define VLAN_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \
 			  VIRTCHNL_VF_OFFLOAD_VLAN)
 	struct virtchnl_vf_resource *vf_res; /* incl. all VSIs */
 	struct virtchnl_vsi_resource *vsi_res; /* our LAN VSI */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 77d2835..21103bb 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1418,7 +1418,7 @@ static int i40evf_init_rss(struct i40evf_adapter *adapter)
 
 	if (!RSS_PF(adapter)) {
 		/* Enable PCTYPES for RSS, TCP/UDP with IPv4/IPv6 */
-		if (adapter->vf_res->vf_offload_flags &
+		if (adapter->vf_res->vf_cap_flags &
 		    VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2)
 			adapter->hena = I40E_DEFAULT_RSS_HENA_EXPANDED;
 		else
@@ -2372,7 +2372,7 @@ static netdev_features_t i40evf_fix_features(struct net_device *netdev,
 	struct i40evf_adapter *adapter = netdev_priv(netdev);
 
 	features &= ~I40EVF_VLAN_FEATURES;
-	if (adapter->vf_res->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_VLAN)
+	if (adapter->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN)
 		features |= I40EVF_VLAN_FEATURES;
 	return features;
 }
@@ -2459,7 +2459,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 	/* advertise to stack only if offloads for encapsulated packets is
 	 * supported
 	 */
-	if (vfres->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_ENCAP) {
+	if (vfres->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ENCAP) {
 		hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL	|
 				   NETIF_F_GSO_GRE		|
 				   NETIF_F_GSO_GRE_CSUM		|
@@ -2469,7 +2469,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 				   NETIF_F_GSO_PARTIAL		|
 				   0;
 
-		if (!(vfres->vf_offload_flags &
+		if (!(vfres->vf_cap_flags &
 		      VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM))
 			netdev->gso_partial_features |=
 				NETIF_F_GSO_UDP_TUNNEL_CSUM;
@@ -2497,7 +2497,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 	adapter->vsi.work_limit = I40E_DEFAULT_IRQ_WORK;
 	vsi->netdev = adapter->netdev;
 	vsi->qs_handle = adapter->vsi_res->qset_handle;
-	if (vfres->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
+	if (vfres->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
 		adapter->rss_key_size = vfres->rss_key_size;
 		adapter->rss_lut_size = vfres->rss_lut_size;
 	} else {
@@ -2665,7 +2665,7 @@ static void i40evf_init_task(struct work_struct *work)
 	if (err)
 		goto err_sw_init;
 	i40evf_map_rings_to_vectors(adapter);
-	if (adapter->vf_res->vf_offload_flags &
+	if (adapter->vf_res->vf_cap_flags &
 	    VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
 		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;
 
diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index c893b95..becfca2 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -223,7 +223,7 @@ struct virtchnl_vsi_resource {
 
 VIRTCHNL_CHECK_STRUCT_LEN(16, virtchnl_vsi_resource);
 
-/* VF offload flags
+/* VF capability flags
  * VIRTCHNL_VF_OFFLOAD_L2 flag is inclusive of base mode L2 offloads including
  * TX/RX Checksum offloading and TSO for non-tunnelled packets.
  */
@@ -251,7 +251,7 @@ struct virtchnl_vf_resource {
 	u16 max_vectors;
 	u16 max_mtu;
 
-	u32 vf_offload_flags;
+	u32 vf_cap_flags;
 	u32 rss_key_size;
 	u32 rss_lut_size;
 
-- 
2.9.4

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Stefan Assmann]

The current name of vf_offload_flags indicates that the bitmap is
limited to offload related features. Make this more generic by renaming
it to vf_cap_flags, which allows for other capabilities besides
offloading to be added.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 22 +++++++++++-----------
 drivers/net/ethernet/intel/i40evf/i40e_common.c    |  2 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         | 10 +++++-----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 12 ++++++------
 include/linux/avf/virtchnl.h                       |  4 ++--
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 3ef67dc..057c77b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1528,39 +1528,39 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 				  VIRTCHNL_VF_OFFLOAD_RSS_REG |
 				  VIRTCHNL_VF_OFFLOAD_VLAN;
 
-	vfres->vf_offload_flags = VIRTCHNL_VF_OFFLOAD_L2;
+	vfres->vf_cap_flags = VIRTCHNL_VF_OFFLOAD_L2;
 	vsi = pf->vsi[vf->lan_vsi_idx];
 	if (!vsi->info.pvid)
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_VLAN;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_VLAN;
 
 	if (i40e_vf_client_capable(pf, vf->vf_id) &&
 	    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_IWARP)) {
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_IWARP;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_IWARP;
 		set_bit(I40E_VF_STATE_IWARPENA, &vf->vf_states);
 	}
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_PF;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_PF;
 	} else {
 		if ((pf->hw_features & I40E_HW_RSS_AQ_CAPABLE) &&
 		    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_AQ))
-			vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_AQ;
+			vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_AQ;
 		else
-			vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RSS_REG;
+			vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RSS_REG;
 	}
 
 	if (pf->hw_features & I40E_HW_MULTIPLE_TCP_UDP_RSS_PCTYPE) {
 		if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2)
-			vfres->vf_offload_flags |=
+			vfres->vf_cap_flags |=
 				VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2;
 	}
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_ENCAP)
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP;
 
 	if ((pf->hw_features & I40E_HW_OUTER_UDP_CSUM_CAPABLE) &&
 	    (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM))
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM;
 
 	if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_RX_POLLING) {
 		if (pf->flags & I40E_FLAG_MFP_ENABLED) {
@@ -1570,12 +1570,12 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 			aq_ret = I40E_ERR_PARAM;
 			goto err;
 		}
-		vfres->vf_offload_flags |= VIRTCHNL_VF_OFFLOAD_RX_POLLING;
+		vfres->vf_cap_flags |= VIRTCHNL_VF_OFFLOAD_RX_POLLING;
 	}
 
 	if (pf->hw_features & I40E_HW_WB_ON_ITR_CAPABLE) {
 		if (vf->driver_caps & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
-			vfres->vf_offload_flags |=
+			vfres->vf_cap_flags |=
 					VIRTCHNL_VF_OFFLOAD_WB_ON_ITR;
 	}
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index 1dd1938..d69c2e4 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -1104,7 +1104,7 @@ void i40e_vf_parse_hw_config(struct i40e_hw *hw,
 	hw->dev_caps.num_rx_qp = msg->num_queue_pairs;
 	hw->dev_caps.num_tx_qp = msg->num_queue_pairs;
 	hw->dev_caps.num_msix_vectors_vf = msg->max_vectors;
-	hw->dev_caps.dcb = msg->vf_offload_flags &
+	hw->dev_caps.dcb = msg->vf_cap_flags &
 			   VIRTCHNL_VF_OFFLOAD_L2;
 	hw->dev_caps.fcoe = 0;
 	for (i = 0; i < msg->num_vsis; i++) {
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index c89767e..d85e446 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -277,19 +277,19 @@ struct i40evf_adapter {
 	enum virtchnl_link_speed link_speed;
 	enum virtchnl_ops current_op;
 #define CLIENT_ALLOWED(_a) ((_a)->vf_res ? \
-			    (_a)->vf_res->vf_offload_flags & \
+			    (_a)->vf_res->vf_cap_flags & \
 				VIRTCHNL_VF_OFFLOAD_IWARP : \
 			    0)
 #define CLIENT_ENABLED(_a) ((_a)->cinst)
 /* RSS by the PF should be preferred over RSS via other methods. */
-#define RSS_PF(_a) ((_a)->vf_res->vf_offload_flags & \
+#define RSS_PF(_a) ((_a)->vf_res->vf_cap_flags & \
 		    VIRTCHNL_VF_OFFLOAD_RSS_PF)
-#define RSS_AQ(_a) ((_a)->vf_res->vf_offload_flags & \
+#define RSS_AQ(_a) ((_a)->vf_res->vf_cap_flags & \
 		    VIRTCHNL_VF_OFFLOAD_RSS_AQ)
-#define RSS_REG(_a) (!((_a)->vf_res->vf_offload_flags & \
+#define RSS_REG(_a) (!((_a)->vf_res->vf_cap_flags & \
 		       (VIRTCHNL_VF_OFFLOAD_RSS_AQ | \
 			VIRTCHNL_VF_OFFLOAD_RSS_PF)))
-#define VLAN_ALLOWED(_a) ((_a)->vf_res->vf_offload_flags & \
+#define VLAN_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \
 			  VIRTCHNL_VF_OFFLOAD_VLAN)
 	struct virtchnl_vf_resource *vf_res; /* incl. all VSIs */
 	struct virtchnl_vsi_resource *vsi_res; /* our LAN VSI */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 77d2835..21103bb 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1418,7 +1418,7 @@ static int i40evf_init_rss(struct i40evf_adapter *adapter)
 
 	if (!RSS_PF(adapter)) {
 		/* Enable PCTYPES for RSS, TCP/UDP with IPv4/IPv6 */
-		if (adapter->vf_res->vf_offload_flags &
+		if (adapter->vf_res->vf_cap_flags &
 		    VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2)
 			adapter->hena = I40E_DEFAULT_RSS_HENA_EXPANDED;
 		else
@@ -2372,7 +2372,7 @@ static netdev_features_t i40evf_fix_features(struct net_device *netdev,
 	struct i40evf_adapter *adapter = netdev_priv(netdev);
 
 	features &= ~I40EVF_VLAN_FEATURES;
-	if (adapter->vf_res->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_VLAN)
+	if (adapter->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN)
 		features |= I40EVF_VLAN_FEATURES;
 	return features;
 }
@@ -2459,7 +2459,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 	/* advertise to stack only if offloads for encapsulated packets is
 	 * supported
 	 */
-	if (vfres->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_ENCAP) {
+	if (vfres->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_ENCAP) {
 		hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL	|
 				   NETIF_F_GSO_GRE		|
 				   NETIF_F_GSO_GRE_CSUM		|
@@ -2469,7 +2469,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 				   NETIF_F_GSO_PARTIAL		|
 				   0;
 
-		if (!(vfres->vf_offload_flags &
+		if (!(vfres->vf_cap_flags &
 		      VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM))
 			netdev->gso_partial_features |=
 				NETIF_F_GSO_UDP_TUNNEL_CSUM;
@@ -2497,7 +2497,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 	adapter->vsi.work_limit = I40E_DEFAULT_IRQ_WORK;
 	vsi->netdev = adapter->netdev;
 	vsi->qs_handle = adapter->vsi_res->qset_handle;
-	if (vfres->vf_offload_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
+	if (vfres->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
 		adapter->rss_key_size = vfres->rss_key_size;
 		adapter->rss_lut_size = vfres->rss_lut_size;
 	} else {
@@ -2665,7 +2665,7 @@ static void i40evf_init_task(struct work_struct *work)
 	if (err)
 		goto err_sw_init;
 	i40evf_map_rings_to_vectors(adapter);
-	if (adapter->vf_res->vf_offload_flags &
+	if (adapter->vf_res->vf_cap_flags &
 	    VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
 		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;
 
diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index c893b95..becfca2 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -223,7 +223,7 @@ struct virtchnl_vsi_resource {
 
 VIRTCHNL_CHECK_STRUCT_LEN(16, virtchnl_vsi_resource);
 
-/* VF offload flags
+/* VF capability flags
  * VIRTCHNL_VF_OFFLOAD_L2 flag is inclusive of base mode L2 offloads including
  * TX/RX Checksum offloading and TSO for non-tunnelled packets.
  */
@@ -251,7 +251,7 @@ struct virtchnl_vf_resource {
 	u16 max_vectors;
 	u16 max_mtu;
 
-	u32 vf_offload_flags;
+	u32 vf_cap_flags;
 	u32 rss_key_size;
 	u32 rss_lut_size;
 
-- 
2.9.4

RE: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Wyborny, Carolyn]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Stefan Assmann
> Sent: Thursday, June 29, 2017 6:12 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to
> vf_cap_flags in struct virtchnl_vf_resource
> 
> The current name of vf_offload_flags indicates that the bitmap is
> limited to offload related features. Make this more generic by renaming
> it to vf_cap_flags, which allows for other capabilities besides
> offloading to be added.
> 
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---

Hello Stefan,

Thanks for the patch series, but we believe the vf should be ignorant of its trusted status.  That being said, you've definitely pointed out a bug in our implementation so that if a vf ends up being configured as trusted, its unable to change its mac address itself when this is an allowed action for this configuration.  The pf can change the mac address for the vf, but the vf itself cannot, which is not correct.  We are working on an alternate solution that keeps the vf from knowing its trusted status, but its not a simple fix.

Thanks,

Carolyn

Carolyn Wyborny 
Linux Development 
Networking Division 
Intel Corporation 

  

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Wyborny, Carolyn]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf
> Of Stefan Assmann
> Sent: Thursday, June 29, 2017 6:12 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to
> vf_cap_flags in struct virtchnl_vf_resource
> 
> The current name of vf_offload_flags indicates that the bitmap is
> limited to offload related features. Make this more generic by renaming
> it to vf_cap_flags, which allows for other capabilities besides
> offloading to be added.
> 
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---

Hello Stefan,

Thanks for the patch series, but we believe the vf should be ignorant of its trusted status.  That being said, you've definitely pointed out a bug in our implementation so that if a vf ends up being configured as trusted, its unable to change its mac address itself when this is an allowed action for this configuration.  The pf can change the mac address for the vf, but the vf itself cannot, which is not correct.  We are working on an alternate solution that keeps the vf from knowing its trusted status, but its not a simple fix.

Thanks,

Carolyn

Carolyn Wyborny 
Linux Development 
Networking Division 
Intel Corporation 

  

Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Greg Rose]

On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
<carolyn.wyborny@intel.com> wrote:
>
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> > Of Stefan Assmann
> > Sent: Thursday, June 29, 2017 6:12 AM
> > To: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to
> > vf_cap_flags in struct virtchnl_vf_resource
> >
> > The current name of vf_offload_flags indicates that the bitmap is
> > limited to offload related features. Make this more generic by renaming
> > it to vf_cap_flags, which allows for other capabilities besides
> > offloading to be added.
> >
> > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> > ---
>
> Hello Stefan,
>
> Thanks for the patch series, but we believe the vf should be ignorant of its trusted status.

Hi Carolyn,

Might I ask why?

Thanks,

- Greg

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Greg Rose]

On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
<carolyn.wyborny@intel.com> wrote:
>
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf
> > Of Stefan Assmann
> > Sent: Thursday, June 29, 2017 6:12 AM
> > To: intel-wired-lan at lists.osuosl.org
> > Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to
> > vf_cap_flags in struct virtchnl_vf_resource
> >
> > The current name of vf_offload_flags indicates that the bitmap is
> > limited to offload related features. Make this more generic by renaming
> > it to vf_cap_flags, which allows for other capabilities besides
> > offloading to be added.
> >
> > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> > ---
>
> Hello Stefan,
>
> Thanks for the patch series, but we believe the vf should be ignorant of its trusted status.

Hi Carolyn,

Might I ask why?

Thanks,

- Greg

RE: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Wyborny, Carolyn]

> -----Original Message-----
> From: Greg Rose [mailto:gvrose8192@gmail.com]
> Sent: Thursday, July 06, 2017 7:25 AM
> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>
> Cc: Stefan Assmann <sassmann@kpanic.de>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; davem@davemloft.net
> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
> 
> On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
> <carolyn.wyborny@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf
> > > Of Stefan Assmann
> > > Sent: Thursday, June 29, 2017 6:12 AM
> > > To: intel-wired-lan@lists.osuosl.org
> > > Cc: netdev@vger.kernel.org; davem@davemloft.net;
> sassmann@kpanic.de
> > > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
> vf_offload_flags to
> > > vf_cap_flags in struct virtchnl_vf_resource
> > >
> > > The current name of vf_offload_flags indicates that the bitmap is
> > > limited to offload related features. Make this more generic by renaming
> > > it to vf_cap_flags, which allows for other capabilities besides
> > > offloading to be added.
> > >
> > > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> > > ---
> >
> > Hello Stefan,
> >
> > Thanks for the patch series, but we believe the vf should be ignorant of its
> trusted status.
> 
> Hi Carolyn,
> 
> Might I ask why?
> 
> Thanks,
> 
> - Greg

Hello Greg,

The trusted status of the vf is something that pf manages, mostly for security reasons.  The mailbox model of communication between them relies on the pf to have authority over the vf.  In most things, the vf asks permission from the pf for changes in its configuration and this keeps the pf as the gatekeeper between trusted and regular vf's.  However, the issue here is that changing the MAC address from within the vf does not go through the mailbox, so it cannot tell if the vf is trusted or not.   We are working on a redesign of this feature to fix that instead of having the vf sort of know its trusted.  If, for some reason, we are unable to redesign it that way we would reconsider this method instead.

Thanks,

Carolyn

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Wyborny, Carolyn]

> -----Original Message-----
> From: Greg Rose [mailto:gvrose8192 at gmail.com]
> Sent: Thursday, July 06, 2017 7:25 AM
> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>
> Cc: Stefan Assmann <sassmann@kpanic.de>; intel-wired-lan at lists.osuosl.org;
> netdev at vger.kernel.org; davem at davemloft.net
> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
> 
> On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
> <carolyn.wyborny@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf
> > > Of Stefan Assmann
> > > Sent: Thursday, June 29, 2017 6:12 AM
> > > To: intel-wired-lan at lists.osuosl.org
> > > Cc: netdev at vger.kernel.org; davem at davemloft.net;
> sassmann at kpanic.de
> > > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
> vf_offload_flags to
> > > vf_cap_flags in struct virtchnl_vf_resource
> > >
> > > The current name of vf_offload_flags indicates that the bitmap is
> > > limited to offload related features. Make this more generic by renaming
> > > it to vf_cap_flags, which allows for other capabilities besides
> > > offloading to be added.
> > >
> > > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> > > ---
> >
> > Hello Stefan,
> >
> > Thanks for the patch series, but we believe the vf should be ignorant of its
> trusted status.
> 
> Hi Carolyn,
> 
> Might I ask why?
> 
> Thanks,
> 
> - Greg

Hello Greg,

The trusted status of the vf is something that pf manages, mostly for security reasons.  The mailbox model of communication between them relies on the pf to have authority over the vf.  In most things, the vf asks permission from the pf for changes in its configuration and this keeps the pf as the gatekeeper between trusted and regular vf's.  However, the issue here is that changing the MAC address from within the vf does not go through the mailbox, so it cannot tell if the vf is trusted or not.   We are working on a redesign of this feature to fix that instead of having the vf sort of know its trusted.  If, for some reason, we are unable to redesign it that way we would reconsider this method instead.

Thanks,

Carolyn

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Stefan Assmann]

On 06.07.2017 17:26, Wyborny, Carolyn wrote:
>> -----Original Message-----
>> From: Greg Rose [mailto:gvrose8192 at gmail.com]
>> Sent: Thursday, July 06, 2017 7:25 AM
>> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>
>> Cc: Stefan Assmann <sassmann@kpanic.de>; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; davem at davemloft.net
>> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
>>
>> On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
>> <carolyn.wyborny@intel.com> wrote:
>>>
>>>> -----Original Message-----
>>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>> Behalf
>>>> Of Stefan Assmann
>>>> Sent: Thursday, June 29, 2017 6:12 AM
>>>> To: intel-wired-lan at lists.osuosl.org
>>>> Cc: netdev at vger.kernel.org; davem at davemloft.net;
>> sassmann at kpanic.de
>>>> Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to
>>>> vf_cap_flags in struct virtchnl_vf_resource
>>>>
>>>> The current name of vf_offload_flags indicates that the bitmap is
>>>> limited to offload related features. Make this more generic by renaming
>>>> it to vf_cap_flags, which allows for other capabilities besides
>>>> offloading to be added.
>>>>
>>>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>>>> ---
>>>
>>> Hello Stefan,
>>>
>>> Thanks for the patch series, but we believe the vf should be ignorant of its
>> trusted status.
>>
>> Hi Carolyn,
>>
>> Might I ask why?
>>
>> Thanks,
>>
>> - Greg
> 
> Hello Greg,
> 
> The trusted status of the vf is something that pf manages, mostly for security reasons.  The mailbox model of communication between them relies on the pf to have authority over the vf.  In most things, the vf asks permission from the pf for changes in its configuration and this keeps the pf as the gatekeeper between trusted and regular vf's.  However, the issue here is that changing the MAC address from within the vf does not go through the mailbox, so it cannot tell if the vf is trusted or not.   We are working on a redesign of this feature to fix that instead of having the vf sort of know its trusted.  If, for some reason, we are unable to redesign it that way we would reconsider this method instead.

Hi Carolyn,

the only other solution I can think of is to use the mailbox interface,
which allows the PF to immediately allow or deny the MAC address change.
If you can make that happen, it would indeed be a better solution.

Independently of that decision, I'd still like to propose patch 1/2
to be applied now. vf_offload_flags is used in struct virtchnl_vf_resource
which is found in the generic header include/linux/avf/virtchnl.h.
So we should keep it as generic as possible.

Let's make the change while the struct hasn't gained popularity
outside of i40e/i40evf.

Thanks!

  Stefan

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Wyborny, Carolyn]

> -----Original Message-----
> From: Stefan Assmann [mailto:sassmann at kpanic.de]
> Sent: Friday, July 07, 2017 1:04 AM
> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>; Greg Rose
> <gvrose8192@gmail.com>
> Cc: intel-wired-lan at lists.osuosl.org; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; davem at davemloft.net
> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
> 
[..]
> 
> Hi Carolyn,
> 
> the only other solution I can think of is to use the mailbox interface,
> which allows the PF to immediately allow or deny the MAC address change.
> If you can make that happen, it would indeed be a better solution.

Yes, a more thorough review of the code shows that we do check with the PF to allow or deny the change as part of the add filter function call.  However, today that does not happen if the PF has set the MAC address for the VF, trusted or not.  Ixgbe and i40e are different in this regard and there is some difference of opinion internally on the best approach, but we are actively working on it.
> 
> Independently of that decision, I'd still like to propose patch 1/2
> to be applied now. vf_offload_flags is used in struct virtchnl_vf_resource
> which is found in the generic header include/linux/avf/virtchnl.h.
> So we should keep it as generic as possible.
> 
> Let's make the change while the struct hasn't gained popularity
> outside of i40e/i40evf.

I'm fine with this change.

Thanks,

Carolyn

Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Greg Rose]

On Thu, Jul 6, 2017 at 4:26 PM, Wyborny, Carolyn
<carolyn.wyborny@intel.com> wrote:
>> -----Original Message-----
>> From: Greg Rose [mailto:gvrose8192@gmail.com]
>> Sent: Thursday, July 06, 2017 7:25 AM
>> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>
>> Cc: Stefan Assmann <sassmann@kpanic.de>; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; davem@davemloft.net
>> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
>>
>> On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
>> <carolyn.wyborny@intel.com> wrote:
>> >
>> > > -----Original Message-----
>> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
>> Behalf
>> > > Of Stefan Assmann
>> > > Sent: Thursday, June 29, 2017 6:12 AM
>> > > To: intel-wired-lan@lists.osuosl.org
>> > > Cc: netdev@vger.kernel.org; davem@davemloft.net;
>> sassmann@kpanic.de
>> > > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to
>> > > vf_cap_flags in struct virtchnl_vf_resource
>> > >
>> > > The current name of vf_offload_flags indicates that the bitmap is
>> > > limited to offload related features. Make this more generic by renaming
>> > > it to vf_cap_flags, which allows for other capabilities besides
>> > > offloading to be added.
>> > >
>> > > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> > > ---
>> >
>> > Hello Stefan,
>> >
>> > Thanks for the patch series, but we believe the vf should be ignorant of its
>> trusted status.
>>
>> Hi Carolyn,
>>
>> Might I ask why?
>>
>> Thanks,
>>
>> - Greg
>
> Hello Greg,
>
> The trusted status of the vf is something that pf manages, mostly for security reasons.  The mailbox model of communication between them relies on the pf to have authority over the vf.  In most things, the vf asks permission from the pf for changes in its configuration and this keeps the pf as the gatekeeper between trusted and regular vf's.  However, the issue here is that changing the MAC address from within the vf does not go through the mailbox, so it cannot tell if the vf is trusted or not.   We are working on a redesign of this feature to fix that instead of having the vf sort of know its trusted.  If, for some reason, we are unable to redesign it that way we would reconsider this method instead.
>
> Thanks,

Thanks, that was my concern, that by allowing the VF to set it's own
MAC then it would implicitly know it was trusted.

Regards,

- Greg
>
> Carolyn

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Greg Rose]

On Thu, Jul 6, 2017 at 4:26 PM, Wyborny, Carolyn
<carolyn.wyborny@intel.com> wrote:
>> -----Original Message-----
>> From: Greg Rose [mailto:gvrose8192 at gmail.com]
>> Sent: Thursday, July 06, 2017 7:25 AM
>> To: Wyborny, Carolyn <carolyn.wyborny@intel.com>
>> Cc: Stefan Assmann <sassmann@kpanic.de>; intel-wired-lan at lists.osuosl.org;
>> netdev at vger.kernel.org; davem at davemloft.net
>> Subject: Re: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource
>>
>> On Wed, Jul 5, 2017 at 10:15 PM, Wyborny, Carolyn
>> <carolyn.wyborny@intel.com> wrote:
>> >
>> > > -----Original Message-----
>> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>> Behalf
>> > > Of Stefan Assmann
>> > > Sent: Thursday, June 29, 2017 6:12 AM
>> > > To: intel-wired-lan at lists.osuosl.org
>> > > Cc: netdev at vger.kernel.org; davem at davemloft.net;
>> sassmann at kpanic.de
>> > > Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename
>> vf_offload_flags to
>> > > vf_cap_flags in struct virtchnl_vf_resource
>> > >
>> > > The current name of vf_offload_flags indicates that the bitmap is
>> > > limited to offload related features. Make this more generic by renaming
>> > > it to vf_cap_flags, which allows for other capabilities besides
>> > > offloading to be added.
>> > >
>> > > Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> > > ---
>> >
>> > Hello Stefan,
>> >
>> > Thanks for the patch series, but we believe the vf should be ignorant of its
>> trusted status.
>>
>> Hi Carolyn,
>>
>> Might I ask why?
>>
>> Thanks,
>>
>> - Greg
>
> Hello Greg,
>
> The trusted status of the vf is something that pf manages, mostly for security reasons.  The mailbox model of communication between them relies on the pf to have authority over the vf.  In most things, the vf asks permission from the pf for changes in its configuration and this keeps the pf as the gatekeeper between trusted and regular vf's.  However, the issue here is that changing the MAC address from within the vf does not go through the mailbox, so it cannot tell if the vf is trusted or not.   We are working on a redesign of this feature to fix that instead of having the vf sort of know its trusted.  If, for some reason, we are unable to redesign it that way we would reconsider this method instead.
>
> Thanks,

Thanks, that was my concern, that by allowing the VF to set it's own
MAC then it would implicitly know it was trusted.

Regards,

- Greg
>
> Carolyn

[Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags to vf_cap_flags in struct virtchnl_vf_resource

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Stefan Assmann
> Sent: Thursday, June 29, 2017 6:12 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH 1/2] i40e/i40evf: rename vf_offload_flags
> to vf_cap_flags in struct virtchnl_vf_resource
> 
> The current name of vf_offload_flags indicates that the bitmap is limited to
> offload related features. Make this more generic by renaming it to
> vf_cap_flags, which allows for other capabilities besides offloading to be
> added.
> 
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 22 +++++++++++--------
> ---
>  drivers/net/ethernet/intel/i40evf/i40e_common.c    |  2 +-
>  drivers/net/ethernet/intel/i40evf/i40evf.h         | 10 +++++-----
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 12 ++++++------
>  include/linux/avf/virtchnl.h                       |  4 ++--
>  5 files changed, 25 insertions(+), 25 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[PATCH 2/2] i40e/i40evf: add virtchnl flag to indicate trusted VF

[Stefan Assmann]

Give the VF a hint whether it runs in trusted mode or not. This allows
the VF to decide if it should allow changing the MAC address.
Currently the VF returns -EPERM in i40evf_set_mac when an
administratively set MAC has been set. However this prevents overriding
the administratively set MAC in trusted mode. Simply removing the EPERM
check is not an option, as that would bring the VF in an inconsistent
state when the PF decides to deny adding the appropriate filter.

Due to the fact that the communication between PF and VF is async the VF
driver cannot decide whether to deny or allow a MAC address change
without the VF trust hint.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 3 ++-
 include/linux/avf/virtchnl.h                       | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 057c77b..9e872b1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1579,6 +1579,9 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 					VIRTCHNL_VF_OFFLOAD_WB_ON_ITR;
 	}
 
+	if (test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps))
+		vfres->vf_cap_flags |= VIRTCHNL_VF_TRUSTED;
+
 	vfres->num_vsis = num_vsis;
 	vfres->num_queue_pairs = vf->num_queue_pairs;
 	vfres->max_vectors = pf->hw.func_caps.num_msix_vectors_vf;
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 21103bb..8822062 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2649,7 +2649,8 @@ static void i40evf_init_task(struct work_struct *work)
 		eth_hw_addr_random(netdev);
 		ether_addr_copy(adapter->hw.mac.addr, netdev->dev_addr);
 	} else {
-		adapter->flags |= I40EVF_FLAG_ADDR_SET_BY_PF;
+		if (!(adapter->vf_res->vf_cap_flags & VIRTCHNL_VF_TRUSTED))
+			adapter->flags |= I40EVF_FLAG_ADDR_SET_BY_PF;
 		ether_addr_copy(netdev->dev_addr, adapter->hw.mac.addr);
 		ether_addr_copy(netdev->perm_addr, adapter->hw.mac.addr);
 	}
diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index becfca2..6d74453 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -240,6 +240,7 @@ VIRTCHNL_CHECK_STRUCT_LEN(16, virtchnl_vsi_resource);
 #define VIRTCHNL_VF_OFFLOAD_ENCAP		0X00100000
 #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM		0X00200000
 #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM	0X00400000
+#define VIRTCHNL_VF_TRUSTED			0X00800000
 
 #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
 			       VIRTCHNL_VF_OFFLOAD_VLAN | \
-- 
2.9.4

[Intel-wired-lan] [PATCH 2/2] i40e/i40evf: add virtchnl flag to indicate trusted VF

[Stefan Assmann]

Give the VF a hint whether it runs in trusted mode or not. This allows
the VF to decide if it should allow changing the MAC address.
Currently the VF returns -EPERM in i40evf_set_mac when an
administratively set MAC has been set. However this prevents overriding
the administratively set MAC in trusted mode. Simply removing the EPERM
check is not an option, as that would bring the VF in an inconsistent
state when the PF decides to deny adding the appropriate filter.

Due to the fact that the communication between PF and VF is async the VF
driver cannot decide whether to deny or allow a MAC address change
without the VF trust hint.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 3 ++-
 include/linux/avf/virtchnl.h                       | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 057c77b..9e872b1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1579,6 +1579,9 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
 					VIRTCHNL_VF_OFFLOAD_WB_ON_ITR;
 	}
 
+	if (test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps))
+		vfres->vf_cap_flags |= VIRTCHNL_VF_TRUSTED;
+
 	vfres->num_vsis = num_vsis;
 	vfres->num_queue_pairs = vf->num_queue_pairs;
 	vfres->max_vectors = pf->hw.func_caps.num_msix_vectors_vf;
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 21103bb..8822062 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2649,7 +2649,8 @@ static void i40evf_init_task(struct work_struct *work)
 		eth_hw_addr_random(netdev);
 		ether_addr_copy(adapter->hw.mac.addr, netdev->dev_addr);
 	} else {
-		adapter->flags |= I40EVF_FLAG_ADDR_SET_BY_PF;
+		if (!(adapter->vf_res->vf_cap_flags & VIRTCHNL_VF_TRUSTED))
+			adapter->flags |= I40EVF_FLAG_ADDR_SET_BY_PF;
 		ether_addr_copy(netdev->dev_addr, adapter->hw.mac.addr);
 		ether_addr_copy(netdev->perm_addr, adapter->hw.mac.addr);
 	}
diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index becfca2..6d74453 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -240,6 +240,7 @@ VIRTCHNL_CHECK_STRUCT_LEN(16, virtchnl_vsi_resource);
 #define VIRTCHNL_VF_OFFLOAD_ENCAP		0X00100000
 #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM		0X00200000
 #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM	0X00400000
+#define VIRTCHNL_VF_TRUSTED			0X00800000
 
 #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
 			       VIRTCHNL_VF_OFFLOAD_VLAN | \
-- 
2.9.4