Re: [EXT] Re: [PATCH net-next] net: mvpp2: extend mib-fragments name to mib-fragments-err
On Thu, Jan 14, 2021 at 04:13:23PM +, Stefan Chulski wrote:
> > > From: Stefan Chulski
> > >
> > > This patch doesn't change any functionality, but just extend MIB
> > > counter register and ethtool-statistic names with "err".
> > >
> > > The counter MVPP2_MIB_FRAGMENTS_RCVD in fact is Error counter.
> > > Extend REG name and appropriated ethtool statistic reg-name with the
> > > ERR/err.
> >
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -1566,7 +1566,7 @@ static u32 mvpp2_read_index(struct mvpp2
> > *priv, u32 index, u32 reg)
> > > { MVPP2_MIB_FC_RCVD, "fc_received" },
> > > { MVPP2_MIB_RX_FIFO_OVERRUN, "rx_fifo_overrun" },
> > > { MVPP2_MIB_UNDERSIZE_RCVD, "undersize_received" },
> > > - { MVPP2_MIB_FRAGMENTS_RCVD, "fragments_received" },
> > > + { MVPP2_MIB_FRAGMENTS_ERR_RCVD, "fragments_err_received" },
> >
> > Hi Stefan
> >
> > I suspect this is now ABI and you cannot change it. You at least need to
> > argue
> > why it is not ABI.
> >
> > Andrew
>
> Hi Andrew,
>
> I not familiar with ABI concept. Does this mean we cannot change, fix or
> extend driver ethtool counters?
As Jakub pointed out, there could be user space looking for this name.
What you could do is add fragments_err_received in addition to
fragments_received. That should not break anything.
Andrew
Re: [EXT] Re: [PATCH net-next] net: mvpp2: extend mib-fragments name to mib-fragments-err
On Thu, 14 Jan 2021 16:13:23 + Stefan Chulski wrote:
> > > From: Stefan Chulski
> > >
> > > This patch doesn't change any functionality, but just extend MIB
> > > counter register and ethtool-statistic names with "err".
> > >
> > > The counter MVPP2_MIB_FRAGMENTS_RCVD in fact is Error counter.
> > > Extend REG name and appropriated ethtool statistic reg-name with the
> > > ERR/err.
> >
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -1566,7 +1566,7 @@ static u32 mvpp2_read_index(struct mvpp2
> > *priv, u32 index, u32 reg)
> > > { MVPP2_MIB_FC_RCVD, "fc_received" },
> > > { MVPP2_MIB_RX_FIFO_OVERRUN, "rx_fifo_overrun" },
> > > { MVPP2_MIB_UNDERSIZE_RCVD, "undersize_received" },
> > > - { MVPP2_MIB_FRAGMENTS_RCVD, "fragments_received" },
> > > + { MVPP2_MIB_FRAGMENTS_ERR_RCVD, "fragments_err_received" },
> >
> > Hi Stefan
> >
> > I suspect this is now ABI and you cannot change it. You at least need to
> > argue
> > why it is not ABI.
> >
> > Andrew
>
> Hi Andrew,
>
> I not familiar with ABI concept. Does this mean we cannot change, fix
> or extend driver ethtool counters?
Extend yes, but some user may have a script tracking
"fragments_received", that script would break if you
rename it.
It'd be good if the hardware errors were reported in
standard netdev statistics.
RE: [EXT] Re: [PATCH net-next] net: mvpp2: extend mib-fragments name to mib-fragments-err
> > From: Stefan Chulski
> >
> > This patch doesn't change any functionality, but just extend MIB
> > counter register and ethtool-statistic names with "err".
> >
> > The counter MVPP2_MIB_FRAGMENTS_RCVD in fact is Error counter.
> > Extend REG name and appropriated ethtool statistic reg-name with the
> > ERR/err.
>
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -1566,7 +1566,7 @@ static u32 mvpp2_read_index(struct mvpp2
> *priv, u32 index, u32 reg)
> > { MVPP2_MIB_FC_RCVD, "fc_received" },
> > { MVPP2_MIB_RX_FIFO_OVERRUN, "rx_fifo_overrun" },
> > { MVPP2_MIB_UNDERSIZE_RCVD, "undersize_received" },
> > - { MVPP2_MIB_FRAGMENTS_RCVD, "fragments_received" },
> > + { MVPP2_MIB_FRAGMENTS_ERR_RCVD, "fragments_err_received" },
>
> Hi Stefan
>
> I suspect this is now ABI and you cannot change it. You at least need to argue
> why it is not ABI.
>
> Andrew
Hi Andrew,
I not familiar with ABI concept. Does this mean we cannot change, fix or extend
driver ethtool counters?
Thanks,
Stefan.
