| Message ID | 20230905180837.3611383-5-ahmed.zaki@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support rx-fcs on/off for VFs | expand |
Dear Ahmed, dear Nobert, Am 05.09.23 um 20:08 schrieb Ahmed Zaki: > From: Norbert Zulinski <norbertx.zulinski@intel.com> > > Previously CRC stripping was always enabled for VF. > > Now it is possible to turn off CRC stripping via ethtool. > ethtool -K <interface> rx-fcs off (Personally, I’d add a blank line before and after a command, and also only indent it by four spaces (similar to Markdown style.) > To turn off CRC stripping, first vlan stripping must be disabled. Maybe also add the corresponding comment. > In iavf_configure_queues add check if CRC stripping is enabled for > VF, if it's enabled then set crc_disabled to false on every VF's > queue. In iavf_set_features add check if CRC stripping setting was > changed then schedule reset. > > Signed-off-by: Norbert Zulinski <norbertx.zulinski@intel.com> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> > --- > drivers/net/ethernet/intel/iavf/iavf.h | 2 + > drivers/net/ethernet/intel/iavf/iavf_main.c | 59 ++++++++++++++++++- > .../net/ethernet/intel/iavf/iavf_virtchnl.c | 4 ++ > include/uapi/linux/ethtool.h | 3 +- > 4 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h > index 738e25657c6b..f32b0453584f 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf.h > +++ b/drivers/net/ethernet/intel/iavf/iavf.h > @@ -406,6 +406,8 @@ struct iavf_adapter { > VIRTCHNL_VF_OFFLOAD_VLAN) > #define VLAN_V2_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ > VIRTCHNL_VF_OFFLOAD_VLAN_V2) > +#define CRC_OFFLOAD_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ > + VIRTCHNL_VF_OFFLOAD_CRC) > #define VLAN_V2_FILTERING_ALLOWED(_a) \ > (VLAN_V2_ALLOWED((_a)) && \ > ((_a)->vlan_v2_caps.filtering.filtering_support.outer || \ > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c > index 0302e46e7942..ed4666b59ad2 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > @@ -4402,6 +4402,9 @@ static int iavf_set_features(struct net_device *netdev, > (features & NETIF_VLAN_OFFLOAD_FEATURES)) > iavf_set_vlan_offload_features(adapter, netdev->features, > features); > + if (CRC_OFFLOAD_ALLOWED(adapter) && > + ((netdev->features & NETIF_F_RXFCS) ^ (features & NETIF_F_RXFCS))) > + iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED); > > return 0; > } > @@ -4523,6 +4526,9 @@ iavf_get_netdev_vlan_hw_features(struct iavf_adapter *adapter) > } > } > > + if (CRC_OFFLOAD_ALLOWED(adapter)) > + hw_features |= NETIF_F_RXFCS; > + > return hw_features; > } > > @@ -4686,6 +4692,55 @@ iavf_fix_netdev_vlan_features(struct iavf_adapter *adapter, > return requested_features; > } > > +/** > + * iavf_fix_strip_features - fix NETDEV strip features based on functionality > + * @adapter: board private structure > + * @requested_features: stack requested NETDEV features > + * > + * Returns fixed-up features bits A better description would be nice. Currently it only seems to match CRC and VLAN stripping. Kind regards, Paul > + **/ > +static netdev_features_t > +iavf_fix_strip_features(struct iavf_adapter *adapter, > + netdev_features_t requested_features) > +{ > + struct net_device *netdev = adapter->netdev; > + bool crc_offload_req, is_vlan_strip; > + netdev_features_t vlan_strip; > + int num_non_zero_vlan; > + > + crc_offload_req = CRC_OFFLOAD_ALLOWED(adapter) && > + (requested_features & NETIF_F_RXFCS); > + num_non_zero_vlan = iavf_get_num_vlans_added(adapter); > + vlan_strip = (NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX); > + is_vlan_strip = requested_features & vlan_strip; > + > + if (!crc_offload_req) > + return requested_features; > + > + if (!num_non_zero_vlan && (netdev->features & vlan_strip) && > + !(netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { > + requested_features &= ~vlan_strip; > + netdev_info(netdev, "Disabling VLAN stripping as FCS/CRC stripping is also disabled and there is no VLAN configured\n"); > + return requested_features; > + } > + > + if ((netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { > + requested_features &= ~vlan_strip; > + if (!(netdev->features & vlan_strip)) > + netdev_info(netdev, "To enable VLAN stripping, first need to enable FCS/CRC stripping"); > + > + return requested_features; > + } > + > + if (num_non_zero_vlan && is_vlan_strip && > + !(netdev->features & NETIF_F_RXFCS)) { > + requested_features &= ~NETIF_F_RXFCS; > + netdev_info(netdev, "To disable FCS/CRC stripping, first need to disable VLAN stripping"); > + } > + > + return requested_features; > +} > + > /** > * iavf_fix_features - fix up the netdev feature bits > * @netdev: our net device > @@ -4698,7 +4753,9 @@ static netdev_features_t iavf_fix_features(struct net_device *netdev, > { > struct iavf_adapter *adapter = netdev_priv(netdev); > > - return iavf_fix_netdev_vlan_features(adapter, features); > + features = iavf_fix_netdev_vlan_features(adapter, features); > + > + return iavf_fix_strip_features(adapter, features); > } > > static const struct net_device_ops iavf_netdev_ops = { > diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c > index 0b97b424e487..8ce6389b5815 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c > @@ -142,6 +142,7 @@ int iavf_send_vf_config_msg(struct iavf_adapter *adapter) > VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2 | > VIRTCHNL_VF_OFFLOAD_ENCAP | > VIRTCHNL_VF_OFFLOAD_VLAN_V2 | > + VIRTCHNL_VF_OFFLOAD_CRC | > VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM | > VIRTCHNL_VF_OFFLOAD_REQ_QUEUES | > VIRTCHNL_VF_OFFLOAD_ADQ | > @@ -312,6 +313,9 @@ void iavf_configure_queues(struct iavf_adapter *adapter) > vqpi->rxq.databuffer_size = > ALIGN(adapter->rx_rings[i].rx_buf_len, > BIT_ULL(IAVF_RXQ_CTX_DBUFF_SHIFT)); > + if (CRC_OFFLOAD_ALLOWED(adapter)) > + vqpi->rxq.crc_disable = !!(adapter->netdev->features & > + NETIF_F_RXFCS); > vqpi++; > } > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index f7fba0dc87e5..adb252fc818a 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -1285,7 +1285,8 @@ struct ethtool_rxfh { > __u32 indir_size; > __u32 key_size; > __u8 hfunc; > - __u8 rsvd8[3]; > + __u8 symm_opts; > + __u8 rsvd8[2]; > __u32 rsvd32; > __u32 rss_config[]; > };
[To: -Norbert due to Recipient address rejected] Am 06.09.23 um 05:57 schrieb Paul Menzel: > Dear Ahmed, dear Nobert, > > > Am 05.09.23 um 20:08 schrieb Ahmed Zaki: >> From: Norbert Zulinski <norbertx.zulinski@intel.com> >> >> Previously CRC stripping was always enabled for VF. >> >> Now it is possible to turn off CRC stripping via ethtool. >> ethtool -K <interface> rx-fcs off > > (Personally, I’d add a blank line before and after a command, and also > only indent it by four spaces (similar to Markdown style.) > >> To turn off CRC stripping, first vlan stripping must be disabled. > > Maybe also add the corresponding comment. > >> In iavf_configure_queues add check if CRC stripping is enabled for >> VF, if it's enabled then set crc_disabled to false on every VF's >> queue. In iavf_set_features add check if CRC stripping setting was >> changed then schedule reset. >> >> Signed-off-by: Norbert Zulinski <norbertx.zulinski@intel.com> >> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> >> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> >> --- >> drivers/net/ethernet/intel/iavf/iavf.h | 2 + >> drivers/net/ethernet/intel/iavf/iavf_main.c | 59 ++++++++++++++++++- >> .../net/ethernet/intel/iavf/iavf_virtchnl.c | 4 ++ >> include/uapi/linux/ethtool.h | 3 +- >> 4 files changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h >> b/drivers/net/ethernet/intel/iavf/iavf.h >> index 738e25657c6b..f32b0453584f 100644 >> --- a/drivers/net/ethernet/intel/iavf/iavf.h >> +++ b/drivers/net/ethernet/intel/iavf/iavf.h >> @@ -406,6 +406,8 @@ struct iavf_adapter { >> VIRTCHNL_VF_OFFLOAD_VLAN) >> #define VLAN_V2_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ >> VIRTCHNL_VF_OFFLOAD_VLAN_V2) >> +#define CRC_OFFLOAD_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ >> + VIRTCHNL_VF_OFFLOAD_CRC) >> #define VLAN_V2_FILTERING_ALLOWED(_a) \ >> (VLAN_V2_ALLOWED((_a)) && \ >> ((_a)->vlan_v2_caps.filtering.filtering_support.outer || \ >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c >> b/drivers/net/ethernet/intel/iavf/iavf_main.c >> index 0302e46e7942..ed4666b59ad2 100644 >> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c >> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c >> @@ -4402,6 +4402,9 @@ static int iavf_set_features(struct net_device >> *netdev, >> (features & NETIF_VLAN_OFFLOAD_FEATURES)) >> iavf_set_vlan_offload_features(adapter, netdev->features, >> features); >> + if (CRC_OFFLOAD_ALLOWED(adapter) && >> + ((netdev->features & NETIF_F_RXFCS) ^ (features & >> NETIF_F_RXFCS))) >> + iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED); >> return 0; >> } >> @@ -4523,6 +4526,9 @@ iavf_get_netdev_vlan_hw_features(struct >> iavf_adapter *adapter) >> } >> } >> + if (CRC_OFFLOAD_ALLOWED(adapter)) >> + hw_features |= NETIF_F_RXFCS; >> + >> return hw_features; >> } >> @@ -4686,6 +4692,55 @@ iavf_fix_netdev_vlan_features(struct >> iavf_adapter *adapter, >> return requested_features; >> } >> +/** >> + * iavf_fix_strip_features - fix NETDEV strip features based on >> functionality >> + * @adapter: board private structure >> + * @requested_features: stack requested NETDEV features >> + * >> + * Returns fixed-up features bits > > A better description would be nice. Currently it only seems to match CRC > and VLAN stripping. > > > Kind regards, > > Paul > > >> + **/ >> +static netdev_features_t >> +iavf_fix_strip_features(struct iavf_adapter *adapter, >> + netdev_features_t requested_features) >> +{ >> + struct net_device *netdev = adapter->netdev; >> + bool crc_offload_req, is_vlan_strip; >> + netdev_features_t vlan_strip; >> + int num_non_zero_vlan; >> + >> + crc_offload_req = CRC_OFFLOAD_ALLOWED(adapter) && >> + (requested_features & NETIF_F_RXFCS); >> + num_non_zero_vlan = iavf_get_num_vlans_added(adapter); >> + vlan_strip = (NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX); >> + is_vlan_strip = requested_features & vlan_strip; >> + >> + if (!crc_offload_req) >> + return requested_features; >> + >> + if (!num_non_zero_vlan && (netdev->features & vlan_strip) && >> + !(netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { >> + requested_features &= ~vlan_strip; >> + netdev_info(netdev, "Disabling VLAN stripping as FCS/CRC >> stripping is also disabled and there is no VLAN configured\n"); >> + return requested_features; >> + } >> + >> + if ((netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { >> + requested_features &= ~vlan_strip; >> + if (!(netdev->features & vlan_strip)) >> + netdev_info(netdev, "To enable VLAN stripping, first need >> to enable FCS/CRC stripping"); >> + >> + return requested_features; >> + } >> + >> + if (num_non_zero_vlan && is_vlan_strip && >> + !(netdev->features & NETIF_F_RXFCS)) { >> + requested_features &= ~NETIF_F_RXFCS; >> + netdev_info(netdev, "To disable FCS/CRC stripping, first need >> to disable VLAN stripping"); >> + } >> + >> + return requested_features; >> +} >> + >> /** >> * iavf_fix_features - fix up the netdev feature bits >> * @netdev: our net device >> @@ -4698,7 +4753,9 @@ static netdev_features_t >> iavf_fix_features(struct net_device *netdev, >> { >> struct iavf_adapter *adapter = netdev_priv(netdev); >> - return iavf_fix_netdev_vlan_features(adapter, features); >> + features = iavf_fix_netdev_vlan_features(adapter, features); >> + >> + return iavf_fix_strip_features(adapter, features); >> } >> static const struct net_device_ops iavf_netdev_ops = { >> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c >> b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c >> index 0b97b424e487..8ce6389b5815 100644 >> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c >> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c >> @@ -142,6 +142,7 @@ int iavf_send_vf_config_msg(struct iavf_adapter >> *adapter) >> VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2 | >> VIRTCHNL_VF_OFFLOAD_ENCAP | >> VIRTCHNL_VF_OFFLOAD_VLAN_V2 | >> + VIRTCHNL_VF_OFFLOAD_CRC | >> VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM | >> VIRTCHNL_VF_OFFLOAD_REQ_QUEUES | >> VIRTCHNL_VF_OFFLOAD_ADQ | >> @@ -312,6 +313,9 @@ void iavf_configure_queues(struct iavf_adapter >> *adapter) >> vqpi->rxq.databuffer_size = >> ALIGN(adapter->rx_rings[i].rx_buf_len, >> BIT_ULL(IAVF_RXQ_CTX_DBUFF_SHIFT)); >> + if (CRC_OFFLOAD_ALLOWED(adapter)) >> + vqpi->rxq.crc_disable = !!(adapter->netdev->features & >> + NETIF_F_RXFCS); >> vqpi++; >> } >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h >> index f7fba0dc87e5..adb252fc818a 100644 >> --- a/include/uapi/linux/ethtool.h >> +++ b/include/uapi/linux/ethtool.h >> @@ -1285,7 +1285,8 @@ struct ethtool_rxfh { >> __u32 indir_size; >> __u32 key_size; >> __u8 hfunc; >> - __u8 rsvd8[3]; >> + __u8 symm_opts; >> + __u8 rsvd8[2]; >> __u32 rsvd32; >> __u32 rss_config[]; >> };
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index 738e25657c6b..f32b0453584f 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -406,6 +406,8 @@ struct iavf_adapter { VIRTCHNL_VF_OFFLOAD_VLAN) #define VLAN_V2_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ VIRTCHNL_VF_OFFLOAD_VLAN_V2) +#define CRC_OFFLOAD_ALLOWED(_a) ((_a)->vf_res->vf_cap_flags & \ + VIRTCHNL_VF_OFFLOAD_CRC) #define VLAN_V2_FILTERING_ALLOWED(_a) \ (VLAN_V2_ALLOWED((_a)) && \ ((_a)->vlan_v2_caps.filtering.filtering_support.outer || \ diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 0302e46e7942..ed4666b59ad2 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -4402,6 +4402,9 @@ static int iavf_set_features(struct net_device *netdev, (features & NETIF_VLAN_OFFLOAD_FEATURES)) iavf_set_vlan_offload_features(adapter, netdev->features, features); + if (CRC_OFFLOAD_ALLOWED(adapter) && + ((netdev->features & NETIF_F_RXFCS) ^ (features & NETIF_F_RXFCS))) + iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED); return 0; } @@ -4523,6 +4526,9 @@ iavf_get_netdev_vlan_hw_features(struct iavf_adapter *adapter) } } + if (CRC_OFFLOAD_ALLOWED(adapter)) + hw_features |= NETIF_F_RXFCS; + return hw_features; } @@ -4686,6 +4692,55 @@ iavf_fix_netdev_vlan_features(struct iavf_adapter *adapter, return requested_features; } +/** + * iavf_fix_strip_features - fix NETDEV strip features based on functionality + * @adapter: board private structure + * @requested_features: stack requested NETDEV features + * + * Returns fixed-up features bits + **/ +static netdev_features_t +iavf_fix_strip_features(struct iavf_adapter *adapter, + netdev_features_t requested_features) +{ + struct net_device *netdev = adapter->netdev; + bool crc_offload_req, is_vlan_strip; + netdev_features_t vlan_strip; + int num_non_zero_vlan; + + crc_offload_req = CRC_OFFLOAD_ALLOWED(adapter) && + (requested_features & NETIF_F_RXFCS); + num_non_zero_vlan = iavf_get_num_vlans_added(adapter); + vlan_strip = (NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_STAG_RX); + is_vlan_strip = requested_features & vlan_strip; + + if (!crc_offload_req) + return requested_features; + + if (!num_non_zero_vlan && (netdev->features & vlan_strip) && + !(netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { + requested_features &= ~vlan_strip; + netdev_info(netdev, "Disabling VLAN stripping as FCS/CRC stripping is also disabled and there is no VLAN configured\n"); + return requested_features; + } + + if ((netdev->features & NETIF_F_RXFCS) && is_vlan_strip) { + requested_features &= ~vlan_strip; + if (!(netdev->features & vlan_strip)) + netdev_info(netdev, "To enable VLAN stripping, first need to enable FCS/CRC stripping"); + + return requested_features; + } + + if (num_non_zero_vlan && is_vlan_strip && + !(netdev->features & NETIF_F_RXFCS)) { + requested_features &= ~NETIF_F_RXFCS; + netdev_info(netdev, "To disable FCS/CRC stripping, first need to disable VLAN stripping"); + } + + return requested_features; +} + /** * iavf_fix_features - fix up the netdev feature bits * @netdev: our net device @@ -4698,7 +4753,9 @@ static netdev_features_t iavf_fix_features(struct net_device *netdev, { struct iavf_adapter *adapter = netdev_priv(netdev); - return iavf_fix_netdev_vlan_features(adapter, features); + features = iavf_fix_netdev_vlan_features(adapter, features); + + return iavf_fix_strip_features(adapter, features); } static const struct net_device_ops iavf_netdev_ops = { diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index 0b97b424e487..8ce6389b5815 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -142,6 +142,7 @@ int iavf_send_vf_config_msg(struct iavf_adapter *adapter) VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2 | VIRTCHNL_VF_OFFLOAD_ENCAP | VIRTCHNL_VF_OFFLOAD_VLAN_V2 | + VIRTCHNL_VF_OFFLOAD_CRC | VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM | VIRTCHNL_VF_OFFLOAD_REQ_QUEUES | VIRTCHNL_VF_OFFLOAD_ADQ | @@ -312,6 +313,9 @@ void iavf_configure_queues(struct iavf_adapter *adapter) vqpi->rxq.databuffer_size = ALIGN(adapter->rx_rings[i].rx_buf_len, BIT_ULL(IAVF_RXQ_CTX_DBUFF_SHIFT)); + if (CRC_OFFLOAD_ALLOWED(adapter)) + vqpi->rxq.crc_disable = !!(adapter->netdev->features & + NETIF_F_RXFCS); vqpi++; } diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index f7fba0dc87e5..adb252fc818a 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -1285,7 +1285,8 @@ struct ethtool_rxfh { __u32 indir_size; __u32 key_size; __u8 hfunc; - __u8 rsvd8[3]; + __u8 symm_opts; + __u8 rsvd8[2]; __u32 rsvd32; __u32 rss_config[]; };