lkml.org 
[lkml]   [2019]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte
Hi Vinicius,

On Wed, 4 Sep 2019 at 00:26, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
>
> Hi,
>
> Vladimir Oltean <olteanv@gmail.com> writes:
>
> > Right. And while we're at it, there's still the potential
> > division-by-zero problem which I still don't know how to solve without
> > implementing a full-blown __ethtool_get_link_ksettings parser that
> > checks against all the possible outputs it can have under the "no
> > carrier" condition - see "[RFC PATCH 1/1] phylink: Set speed to
> > SPEED_UNKNOWN when there is no PHY connected" for details.
> > And there's also a third fix to be made: the netdev_dbg should be made
> > to print "speed" instead of "ecmd.base.speed".
>
> For the ksettings part I am thinking on adding something like this to
> ethtool.c. Do you think anything is missing (apart from the
> documentation)?
>
> ->
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 95991e43..d37c80b 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -177,6 +177,9 @@ void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
> bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
> const unsigned long *src);
>
> +u32 ethtool_link_ksettings_to_speed(const struct ethtool_link_ksettings *settings,
> + u32 default_speed);
> +
> /**
> * struct ethtool_ops - optional netdev operations
> * @get_drvinfo: Report driver/device information. Should only set the
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 6288e69..80e3db3 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -539,6 +539,18 @@ struct ethtool_link_usettings {
> } link_modes;
> };
>
> +u32 ethtool_link_ksettings_to_speed(const struct ethtool_link_ksettings *settings,
> + u32 default_speed)
> +{
> + if (settings->base.speed == SPEED_UNKNOWN)
> + return default_speed;
> +
> + if (settings->base.speed == 0)
> + return default_speed;
> +
> + return settings->base.speed;
> +}
> +
> /* Internal kernel helper to query a device ethtool_link_settings. */
> int __ethtool_get_link_ksettings(struct net_device *dev,
> struct ethtool_link_ksettings *link_ksettings)

Looks ok to me, but I have no saying over ethtool API. Actually I
don't even know whom to ask - the output of
./scripts/get_maintainer.pl net/core/ethtool.c is a bit overwhelming.
To avoid conflicts, there needs to be somebody out of us who takes
Eric's simplification, with Gustavo's Reported-by tag, and the 2
ethtool & taprio patches to avoid division by zero, and the printing
fix, and maybe do the same in cbs. Will you be the one? Should I?

Thanks,
-Vladimir

\
 
 \ /
  Last update: 2019-09-05 22:48    [W:0.074 / U:8.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site