[Intel-wired-lan] [PATCH net v1] iavf: Fix handling of vlan strip virtual channel messages

[Nguyen, Anthony L]

On Thu, 2021-10-07 at 12:56 +0000, Michal Maloszewski wrote:
> Modify netdev->features for vlan stripping based on virtual
> channel messages received from the PF. Change is needed
> to synchronize vlan strip status between PF sysfs and iavf ethtool.


This patch doesn't apply.

> Fixes: iavf: 129cf89e58567 ("rename functions and structs to new
> name")

The Fixes tag is incorrect. It should look like this:

Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the
number of pages it actually freed")

https://www.kernel.org/doc/html/latest/process/submitting-
patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-
fixes

> Signed-off-by: Norbert Ciosek <norbertx.ciosek at intel.com>
> Signed-off-by: Michal Maloszewski <michal.maloszewski at intel.com>
> ---
>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 48
> ++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> index 9c128462e..8e18ae0b5 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> @@ -1456,6 +1456,22 @@ void iavf_request_reset(struct iavf_adapter
> *adapter)
>         adapter->current_op = VIRTCHNL_OP_UNKNOWN;
>  }
>  
> +/**
> + * iavf_netdev_features_vlan_strip_set
> + * @netdev: ptr to netdev being adjusted
> + * @enable: enable or disable vlan strip
> + *
> + * Helper function to change vlan strip status in netdev->features.
> + **/
> +static void iavf_netdev_features_vlan_strip_set(struct net_device
> *netdev,
> +                                              const bool enable)
> +{
> +       if (enable)
> +            netdev->features |= NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_HW_VLAN_RX;
> +       else
> +            netdev->features &= ~NETIF_F_HW_VLAN_CTAG_RX &
> ~NETIF_F_HW_VLAN_RX;
> +}
> +
>  /**
>   * iavf_virtchnl_completion
>   * @adapter: adapter structure
> @@ -1679,8 +1695,22 @@ void iavf_virtchnl_completion(struct
> iavf_adapter *adapter,
>                         }
>                         break;
>                 case VIRTCHNL_OP_ENABLE_VLAN_STRIPPING:
> +                       dev_warn(&adapter->pdev->dev,
> +                                "Changing VLAN Stripping is not
> allowed when Port VLAN is configured\n");

splitting this line isn't needed

> +                       /*
> +                        * Vlan stripping could not be enabled by
> ethtool.
> +                        * Disable it in netdev->features.
> +                        */
> +                       iavf_netdev_features_vlan_strip_set(netdev,
> false);
> +                       break;
>                 case VIRTCHNL_OP_DISABLE_VLAN_STRIPPING:
> -                       dev_warn(&adapter->pdev->dev, "Changing VLAN
> Stripping is not allowed when Port VLAN is configured\n");
> +                       dev_warn(&adapter->pdev->dev,
> +                                "Changing VLAN Stripping is not
> allowed when Port VLAN is configured\n");

Why the change? Strings can be long line

> +                       /*
> +                        * Vlan stripping could not be disabled by
> ethtool.
> +                        * Enable it in netdev->features.
> +                        */
> +                       iavf_netdev_features_vlan_strip_set(netdev,
> true);

This comment style is incorrect for netdev patches.

For files in net/ and drivers/net/ the preferred style for long (multi-
line) comments is a little different.

/* The preferred comment style for files in net/ and drivers/net
 * looks like this.
 *
 * It is nearly the same as the generally preferred comment style,
 * but there is no initial almost-blank line.
 */

https://www.kernel.org/doc/html/latest/process/coding-style.html

>                         break;
>                 default:
>                         dev_err(&adapter->pdev->dev, "PF returned
> error %d (%s) to our request %d\n",
> @@ -1897,6 +1927,22 @@ void iavf_virtchnl_completion(struct
> iavf_adapter *adapter,
>                 spin_unlock_bh(&adapter->adv_rss_lock);
>                 }
>                 break;
> +       case VIRTCHNL_OP_ENABLE_VLAN_STRIPPING:
> +               /*
> +                * Got information that PF enabled vlan strip on this
> VF.
> +                * Update netdev->features if needed to be in sync
> with ethtool.
> +                */

ditto for comment style

> +               if (!v_retval)
> +                       iavf_netdev_features_vlan_strip_set(netdev,
> true);
> +               break;
> +       case VIRTCHNL_OP_DISABLE_VLAN_STRIPPING:
> +               /*
> +                * Got information that PF disabled vlan strip on
> this VF.
> +                * Update netdev->features if needed to be in sync
> with ethtool.
> +                */

ditto for comment style

> +               if (!v_retval)
> +                       iavf_netdev_features_vlan_strip_set(netdev,
> false);
> +               break;
>         default:
>                 if (adapter->current_op && (v_opcode != adapter-
> >current_op))
>                         dev_dbg(&adapter->pdev->dev, "Expected
> response %d from PF, received %d\n",