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] [day] [month] [year] [list]
Message-ID: <CAJ3xEMi-si0g1pZrmQ8B+KVRFGsSSacHq1=byAxnOmZfAaLbhQ@mail.gmail.com>
Date:	Tue, 21 Jun 2016 15:17:23 +0300
From:	Or Gerlitz <gerlitz.or@...il.com>
To:	Tariq Toukan <tariqt@...lanox.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Rana Shahout <ranas@...lanox.com>,
	Eran Ben Elisha <eranbe@...lanox.com>,
	Eugenia Emantayev <eugenia@...lanox.com>
Subject: Re: [PATCH net-next] net/mlx4_en: Add DCB PFC support through CEE
 netlink commands

On Tue, Jun 21, 2016 at 2:12 PM, Tariq Toukan <tariqt@...lanox.com> wrote:
> From: Rana Shahout <ranas@...lanox.com>
>
> This patch adds support for reading and updating priority flow
> control (PFC) attributes in the driver via netlink.

The patch does multiple things, you need to break it to few patches, I
guess 2-3 patches could be fine, e.g

1. changes to areas areas where you read new values from the FW
2. changes/refactoring to the IEEE code to make some of the callbacks general
3. add CEE code and usage of the code you added in steps 1,2 above



> ---
>  drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c | 277 +++++++++++++++++++++++--
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |  25 +++
>  drivers/net/ethernet/mellanox/mlx4/fw.c        |   1 +
>  drivers/net/ethernet/mellanox/mlx4/fw.h        |   1 +
>  drivers/net/ethernet/mellanox/mlx4/main.c      |   1 +
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |  26 +++
>  drivers/net/ethernet/mellanox/mlx4/port.c      |  12 ++
>  include/linux/mlx4/device.h                    |   2 +
>  8 files changed, 331 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
> index f01918c..99c6bbd 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
> @@ -37,6 +37,11 @@
>  #include "mlx4_en.h"
>  #include "fw_qos.h"
>
> +enum {
> +       MLX4_CEE_STATE_DOWN   = 0,
> +       MLX4_CEE_STATE_UP     = 1,
> +};
> +
>  /* Definitions for QCN
>   */
>
> @@ -80,13 +85,202 @@ struct mlx4_congestion_control_mb_prio_802_1_qau_statistics {
>         __be32 reserved3[4];
>  };
>
> +static u8 mlx4_en_dcbnl_getcap(struct net_device *dev, int capid, u8 *cap)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(dev);
> +
> +       switch (capid) {
> +       case DCB_CAP_ATTR_PFC:
> +               *cap = true;
> +               break;
> +       case DCB_CAP_ATTR_DCBX:
> +               *cap = priv->cee_params.dcbx_cap;
> +               break;
> +       case DCB_CAP_ATTR_PFC_TCS:
> +               *cap = 1 <<  mlx4_max_tc(priv->mdev->dev);
> +               break;
> +       default:
> +               *cap = false;
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +static u8 mlx4_en_dcbnl_getpfcstate(struct net_device *netdev)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +
> +       return priv->cee_params.dcb_cfg.pfc_state;
> +}
> +
> +static void mlx4_en_dcbnl_setpfcstate(struct net_device *netdev, u8 state)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +
> +       priv->cee_params.dcb_cfg.pfc_state = state;
> +}
> +
> +static void mlx4_en_dcbnl_get_pfc_cfg(struct net_device *netdev, int priority,
> +                                     u8 *setting)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +
> +       *setting = priv->cee_params.dcb_cfg.tc_config[priority].dcb_pfc;
> +}
> +
> +static void mlx4_en_dcbnl_set_pfc_cfg(struct net_device *netdev, int priority,
> +                                     u8 setting)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +
> +       priv->cee_params.dcb_cfg.tc_config[priority].dcb_pfc = setting;
> +       priv->cee_params.dcb_cfg.pfc_state = true;
> +}
> +
> +static int mlx4_en_dcbnl_getnumtcs(struct net_device *netdev, int tcid, u8 *num)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +
> +       if (!(priv->flags & MLX4_EN_FLAG_DCB_ENABLED))
> +               return -EINVAL;
> +
> +       if (tcid == DCB_NUMTCS_ATTR_PFC)
> +               *num = mlx4_max_tc(priv->mdev->dev);
> +       else
> +               *num = 0;
> +
> +       return 0;
> +}
> +
> +static u8 mlx4_en_dcbnl_set_all(struct net_device *netdev)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +       struct mlx4_en_dev *mdev = priv->mdev;
> +       struct mlx4_en_cee_config *dcb_cfg = &priv->cee_params.dcb_cfg;
> +       int err = 0;
> +
> +       if (!(priv->cee_params.dcbx_cap & DCB_CAP_DCBX_VER_CEE))
> +               return -EINVAL;
> +
> +       if (dcb_cfg->pfc_state) {
> +               int tc;
> +
> +               priv->prof->rx_pause = 0;
> +               priv->prof->tx_pause = 0;
> +               for (tc = 0; tc < CEE_DCBX_MAX_PRIO; tc++) {
> +                       u8 tc_mask = 1 << tc;
> +
> +                       switch (dcb_cfg->tc_config[tc].dcb_pfc) {
> +                       case pfc_disabled:
> +                               priv->prof->tx_ppp &= ~tc_mask;
> +                               priv->prof->rx_ppp &= ~tc_mask;
> +                               break;
> +                       case pfc_enabled_full:
> +                               priv->prof->tx_ppp |= tc_mask;
> +                               priv->prof->rx_ppp |= tc_mask;
> +                               break;
> +                       case pfc_enabled_tx:
> +                               priv->prof->tx_ppp |= tc_mask;
> +                               priv->prof->rx_ppp &= ~tc_mask;
> +                               break;
> +                       case pfc_enabled_rx:
> +                               priv->prof->tx_ppp &= ~tc_mask;
> +                               priv->prof->rx_ppp |= tc_mask;
> +                               break;
> +                       default:
> +                               break;
> +                       }
> +               }
> +               en_dbg(DRV, priv, "Set pfc on\n");
> +       } else {
> +               priv->prof->rx_pause = 1;
> +               priv->prof->tx_pause = 1;
> +               en_dbg(DRV, priv, "Set pfc off\n");
> +       }
> +
> +       err = mlx4_SET_PORT_general(mdev->dev, priv->port,
> +                                   priv->rx_skb_size + ETH_FCS_LEN,
> +                                   priv->prof->tx_pause,
> +                                   priv->prof->tx_ppp,
> +                                   priv->prof->rx_pause,
> +                                   priv->prof->rx_ppp);
> +       if (err)
> +               en_err(priv, "Failed setting pause params\n");
> +       return err;
> +}
> +
> +static u8 mlx4_en_dcbnl_get_state(struct net_device *dev)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(dev);
> +
> +       if (priv->flags & MLX4_EN_FLAG_DCB_ENABLED)
> +               return MLX4_CEE_STATE_UP;
> +
> +       return MLX4_CEE_STATE_DOWN;
> +}
> +
> +static u8 mlx4_en_dcbnl_set_state(struct net_device *dev, u8 state)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(dev);
> +       int num_tcs = 0;
> +
> +       if (!(priv->cee_params.dcbx_cap & DCB_CAP_DCBX_VER_CEE))
> +               return 1;
> +
> +       if (!!(state) == !!(priv->flags & MLX4_EN_FLAG_DCB_ENABLED))
> +               return 0;
> +
> +       if (state) {
> +               priv->flags |= MLX4_EN_FLAG_DCB_ENABLED;
> +               num_tcs = IEEE_8021QAZ_MAX_TCS;
> +       } else {
> +               priv->flags &= ~MLX4_EN_FLAG_DCB_ENABLED;
> +       }
> +
> +       return mlx4_en_setup_tc(dev, num_tcs);
> +}
> +
> +/* On success returns a non-zero 802.1p user priority bitmap
> + * otherwise returns 0 as the invalid user priority bitmap to
> + * indicate an error.
> + */
> +static int mlx4_en_dcbnl_getapp(struct net_device *netdev, u8 idtype, u16 id)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +       struct dcb_app app = {
> +                               .selector = idtype,
> +                               .protocol = id,
> +                            };
> +       if (!(priv->cee_params.dcbx_cap & DCB_CAP_DCBX_VER_CEE))
> +               return 0;
> +
> +       return dcb_getapp(netdev, &app);
> +}
> +
> +static int mlx4_en_dcbnl_setapp(struct net_device *netdev, u8 idtype,
> +                               u16 id, u8 up)
> +{
> +       struct mlx4_en_priv *priv = netdev_priv(netdev);
> +       struct dcb_app app;
> +
> +       if (!(priv->cee_params.dcbx_cap & DCB_CAP_DCBX_VER_CEE))
> +               return -EINVAL;
> +
> +       memset(&app, 0, sizeof(struct dcb_app));
> +       app.selector = idtype;
> +       app.protocol = id;
> +       app.priority = up;
> +
> +       return dcb_setapp(netdev, &app);
> +}
> +
>  static int mlx4_en_dcbnl_ieee_getets(struct net_device *dev,
>                                    struct ieee_ets *ets)
>  {
>         struct mlx4_en_priv *priv = netdev_priv(dev);
>         struct ieee_ets *my_ets = &priv->ets;
>
> -       /* No IEEE PFC settings available */
>         if (!my_ets)
>                 return -EINVAL;
>
> @@ -237,18 +431,51 @@ static int mlx4_en_dcbnl_ieee_setpfc(struct net_device *dev,
>
>  static u8 mlx4_en_dcbnl_getdcbx(struct net_device *dev)
>  {
> -       return DCB_CAP_DCBX_HOST | DCB_CAP_DCBX_VER_IEEE;
> +       struct mlx4_en_priv *priv = netdev_priv(dev);
> +
> +       return priv->cee_params.dcbx_cap;
>  }
>
>  static u8 mlx4_en_dcbnl_setdcbx(struct net_device *dev, u8 mode)
>  {
> +       struct mlx4_en_priv *priv = netdev_priv(dev);
> +       struct ieee_ets ets = {0};
> +       struct ieee_pfc pfc = {0};
> +
> +       if (mode == priv->cee_params.dcbx_cap)
> +               return 0;
> +
>         if ((mode & DCB_CAP_DCBX_LLD_MANAGED) ||
> -           (mode & DCB_CAP_DCBX_VER_CEE) ||
> -           !(mode & DCB_CAP_DCBX_VER_IEEE) ||
> +           ((mode & DCB_CAP_DCBX_VER_IEEE) &&
> +            (mode & DCB_CAP_DCBX_VER_CEE)) ||
>             !(mode & DCB_CAP_DCBX_HOST))
> -               return 1;
> +               goto err;
> +
> +       priv->cee_params.dcbx_cap = mode;
> +
> +       ets.ets_cap = IEEE_8021QAZ_MAX_TCS;
> +       pfc.pfc_cap = IEEE_8021QAZ_MAX_TCS;
> +
> +       if (mode & DCB_CAP_DCBX_VER_IEEE) {
> +               if (mlx4_en_dcbnl_ieee_setets(dev, &ets))
> +                       goto err;
> +               if (mlx4_en_dcbnl_ieee_setpfc(dev, &pfc))
> +                       goto err;
> +       } else if (mode & DCB_CAP_DCBX_VER_CEE) {
> +               if (mlx4_en_dcbnl_set_all(dev))
> +                       goto err;
> +       } else {
> +               if (mlx4_en_dcbnl_ieee_setets(dev, &ets))
> +                       goto err;
> +               if (mlx4_en_dcbnl_ieee_setpfc(dev, &pfc))
> +                       goto err;
> +               if (mlx4_en_setup_tc(dev, 0))
> +                       goto err;
> +       }
>
>         return 0;
> +err:
> +       return 1;
>  }
>
>  #define MLX4_RATELIMIT_UNITS_IN_KB 100000 /* rate-limit HW unit in Kbps */
> @@ -463,24 +690,46 @@ static int mlx4_en_dcbnl_ieee_getqcnstats(struct net_device *dev,
>  }
>
>  const struct dcbnl_rtnl_ops mlx4_en_dcbnl_ops = {
> -       .ieee_getets    = mlx4_en_dcbnl_ieee_getets,
> -       .ieee_setets    = mlx4_en_dcbnl_ieee_setets,
> -       .ieee_getmaxrate = mlx4_en_dcbnl_ieee_getmaxrate,
> -       .ieee_setmaxrate = mlx4_en_dcbnl_ieee_setmaxrate,
> -       .ieee_getpfc    = mlx4_en_dcbnl_ieee_getpfc,
> -       .ieee_setpfc    = mlx4_en_dcbnl_ieee_setpfc,
> +       .ieee_getets            = mlx4_en_dcbnl_ieee_getets,
> +       .ieee_setets            = mlx4_en_dcbnl_ieee_setets,
> +       .ieee_getmaxrate        = mlx4_en_dcbnl_ieee_getmaxrate,
> +       .ieee_setmaxrate        = mlx4_en_dcbnl_ieee_setmaxrate,
> +       .ieee_getqcn            = mlx4_en_dcbnl_ieee_getqcn,
> +       .ieee_setqcn            = mlx4_en_dcbnl_ieee_setqcn,
> +       .ieee_getqcnstats       = mlx4_en_dcbnl_ieee_getqcnstats,
> +       .ieee_getpfc            = mlx4_en_dcbnl_ieee_getpfc,
> +       .ieee_setpfc            = mlx4_en_dcbnl_ieee_setpfc,
> +
> +       .getstate       = mlx4_en_dcbnl_get_state,
> +       .setstate       = mlx4_en_dcbnl_set_state,
> +       .getpfccfg      = mlx4_en_dcbnl_get_pfc_cfg,
> +       .setpfccfg      = mlx4_en_dcbnl_set_pfc_cfg,
> +       .setall         = mlx4_en_dcbnl_set_all,
> +       .getcap         = mlx4_en_dcbnl_getcap,
> +       .getnumtcs      = mlx4_en_dcbnl_getnumtcs,
> +       .getpfcstate    = mlx4_en_dcbnl_getpfcstate,
> +       .setpfcstate    = mlx4_en_dcbnl_setpfcstate,
> +       .getapp         = mlx4_en_dcbnl_getapp,
> +       .setapp         = mlx4_en_dcbnl_setapp,
>
>         .getdcbx        = mlx4_en_dcbnl_getdcbx,
>         .setdcbx        = mlx4_en_dcbnl_setdcbx,
> -       .ieee_getqcn    = mlx4_en_dcbnl_ieee_getqcn,
> -       .ieee_setqcn    = mlx4_en_dcbnl_ieee_setqcn,
> -       .ieee_getqcnstats = mlx4_en_dcbnl_ieee_getqcnstats,
>  };
>
>  const struct dcbnl_rtnl_ops mlx4_en_dcbnl_pfc_ops = {
>         .ieee_getpfc    = mlx4_en_dcbnl_ieee_getpfc,
>         .ieee_setpfc    = mlx4_en_dcbnl_ieee_setpfc,
>
> +       .setstate       = mlx4_en_dcbnl_set_state,
> +       .getpfccfg      = mlx4_en_dcbnl_get_pfc_cfg,
> +       .setpfccfg      = mlx4_en_dcbnl_set_pfc_cfg,
> +       .setall         = mlx4_en_dcbnl_set_all,
> +       .getnumtcs      = mlx4_en_dcbnl_getnumtcs,
> +       .getpfcstate    = mlx4_en_dcbnl_getpfcstate,
> +       .setpfcstate    = mlx4_en_dcbnl_setpfcstate,
> +       .getapp         = mlx4_en_dcbnl_getapp,
> +       .setapp         = mlx4_en_dcbnl_setapp,
> +
>         .getdcbx        = mlx4_en_dcbnl_getdcbx,
>         .setdcbx        = mlx4_en_dcbnl_setdcbx,
>  };
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8e318d2..d42083a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -67,6 +67,17 @@ int mlx4_en_setup_tc(struct net_device *dev, u8 up)
>                 offset += priv->num_tx_rings_p_up;
>         }
>
> +#ifdef CONFIG_MLX4_EN_DCB
> +       if (!mlx4_is_slave(priv->mdev->dev)) {
> +               if (up) {
> +                       priv->flags |= MLX4_EN_FLAG_DCB_ENABLED;

you don't check any device capability here before declaring that, bug, I guess





> diff --git a/drivers/net/ethernet/mellanox/mlx4/port.c b/drivers/net/ethernet/mellanox/mlx4/port.c
> index 087b23b..3d2095e 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/port.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/port.c
> @@ -52,6 +52,7 @@
>
>  #define MLX4_FLAG_V_IGNORE_FCS_MASK            0x2
>  #define MLX4_IGNORE_FCS_MASK                   0x1
> +#define MLNX4_TX_MAX_NUMBER                    8

MLNX --> MLX
TX --> TC ?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ