On Wed,  6 Jan 2021 15:06:16 +0200 Danielle Ratson wrote:
> From: Danielle Ratson <daniel...@nvidia.com>
> 
> Currently, when auto negotiation is on, the user can advertise all the
> linkmodes which correspond to a specific speed, but does not have a
> similar selector for the number of lanes. This is significant when a
> specific speed can be achieved using different number of lanes.  For
> example, 2x50 or 4x25.
> 
> Add 'ETHTOOL_A_LINKMODES_LANES' attribute and expand 'struct
> ethtool_link_settings' with lanes field in order to implement a new
> lanes-selector that will enable the user to advertise a specific number
> of lanes as well.
> 
> When auto negotiation is off, lanes parameter can be forced only if the
> driver supports it. Add a capability bit in 'struct ethtool_ops' that
> allows ethtool know if the driver can handle the lanes parameter when
> auto negotiation is off, so if it does not, an error message will be
> returned when trying to set lanes.

> @@ -420,6 +423,7 @@ struct ethtool_pause_stats {
>   * of the generic netdev features interface.
>   */
>  struct ethtool_ops {
> +     u32     capabilities;

An appropriately named bitfield seems better. Alternatively maybe let
the driver specify which lane counts it can accept?

And please remember to add the kdoc.

>       u32     supported_coalesce_params;
>       void    (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
>       int     (*get_regs_len)(struct net_device *);

> @@ -274,16 +277,17 @@ const struct nla_policy ethnl_linkmodes_set_policy[] = {
>       [ETHTOOL_A_LINKMODES_SPEED]             = { .type = NLA_U32 },
>       [ETHTOOL_A_LINKMODES_DUPLEX]            = { .type = NLA_U8 },
>       [ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]  = { .type = NLA_U8 },
> +     [ETHTOOL_A_LINKMODES_LANES]             = { .type = NLA_U32 },

Please set the min and max for the policy, so userspace can at least
see that part.

> +static bool ethnl_validate_lanes_cfg(u32 cfg)
> +{
> +     switch (cfg) {
> +     case 1:
> +     case 2:
> +     case 4:
> +     case 8:
> +             return true;

And with the policy checking min and max this can be turned into
a simple is_power_of_2() call.

> +     }
> +
> +     return false;
> +}

Reply via email to