Date: Sun, 22 Sep 2024 14:19:54 +0100 From: Warner Losh <imp@bsdimp.com> To: Mark Johnston <markj@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, Bram <bram@cbbg.nl> Subject: Re: git: 94567c8641e2 - main - netstat: Resolve duplicate "dropped-packets" key from libxo output Message-ID: <CANCZdfqAaAGc2ES8gtOX6Tx2WZsSthGdcSxUNtcZWKqYDwKzSQ@mail.gmail.com> In-Reply-To: <Zu_xk5GoyC2ykk7Q@nuc> References: <202409201558.48KFwAEN048834@gitrepo.freebsd.org> <Zu_xk5GoyC2ykk7Q@nuc>
next in thread | previous in thread | raw e-mail | index | archive | help
--00000000000076754d0622b521c7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Sep 22, 2024 at 11:29=E2=80=AFAM Mark Johnston <markj@freebsd.org> = wrote: > On Fri, Sep 20, 2024 at 03:58:10PM +0000, Warner Losh wrote: > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D94567c8641e235763b5b2926416d89d= 36654cde1 > > > > commit 94567c8641e235763b5b2926416d89d36654cde1 > > Author: Bram <bram@cbbg.nl> > > AuthorDate: 2024-07-23 08:57:42 +0000 > > Commit: Warner Losh <imp@FreeBSD.org> > > CommitDate: 2024-09-20 15:06:26 +0000 > > > > netstat: Resolve duplicate "dropped-packets" key from libxo output > > > > The current libxo output uses the "dropped-packets" key for both > incoming and > > outgoing packets. This commit distinguishes between the two by > adding an "in" > > and "out" suffix. The original "dropped-packets" key is kept for > backwards > > compatibility for now. > > > > PR: 244589 > > Reviewed by: imp, zlei > > Pull Request: https://github.com/freebsd/freebsd-src/pull/1331 > > --- > > usr.bin/netstat/if.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/usr.bin/netstat/if.c b/usr.bin/netstat/if.c > > index 172ea5324ccb..f0af785bce04 100644 > > --- a/usr.bin/netstat/if.c > > +++ b/usr.bin/netstat/if.c > > @@ -501,8 +501,11 @@ intpr(void (*pfunc)(char *), int af) > > IFA_STAT(ipackets), link|network, 1); > > show_stat("lu", nerr_len, "received-errors", > IFA_STAT(ierrors), > > link, 1); > > + /* Below is kept for backwards compatibility. Will be > removed in the future. */ > > show_stat("lu", nerr_len, "dropped-packets", > IFA_STAT(iqdrops), > > link, 1); > > + show_stat("lu", nerr_len, "dropped-packets-in", > IFA_STAT(iqdrops), > > + link, 1); > > This breaks plain netstat output, causing a bunch of network tests to > fail. For instance, "netstat -Wf link -I re0" now prints an extra > column. > > I think we should not bother with backwards compat unless libxo is > emitting structured output. This can be implemented using the "e" > format modifier described in the xo_format modified, but I'm not sure > how to plumb it through show_stat(). > > Let's revert until this is fixed? > Works for me. I hadn't noticed before the push. Thanks for bringing it up. Warner > > if (bflag) > > show_stat("lu", nbyte_len, "received-bytes", > > IFA_STAT(ibytes), link|network, 0); > > @@ -516,7 +519,7 @@ intpr(void (*pfunc)(char *), int af) > > show_stat("NRSlu", nerr_len, "collisions", > IFA_STAT(collisions), > > link, 1); > > if (dflag) > > - show_stat("LSlu", nerr_len, "dropped-packets", > > + show_stat("LSlu", nerr_len, "dropped-packets-out"= , > > IFA_STAT(oqdrops), link, 1); > > xo_emit("\n"); > > > > @@ -705,8 +708,11 @@ loop: > > new->ift_ip - old->ift_ip, 1, 1); > > show_stat("lu", 5, "received-errors", > > new->ift_ie - old->ift_ie, 1, 1); > > + /* Below is kept for backwards compatibility. Will be removed in > the future. */ > > show_stat("lu", 5, "dropped-packets", > > new->ift_id - old->ift_id, 1, 1); > > + show_stat("lu", 5, "dropped-packets-in", > > + new->ift_id - old->ift_id, 1, 1); > > show_stat("lu", 10, "received-bytes", > > new->ift_ib - old->ift_ib, 1, 0); > > show_stat("lu", 10, "sent-packets", > > @@ -718,7 +724,7 @@ loop: > > show_stat("NRSlu", 5, "collisions", > > new->ift_co - old->ift_co, 1, 1); > > if (dflag) > > - show_stat("LSlu", 5, "dropped-packets", > > + show_stat("LSlu", 5, "dropped-packets-out", > > new->ift_od - old->ift_od, 1, 1); > > xo_close_instance("stats"); > > xo_emit("\n"); > --00000000000076754d0622b521c7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Sun, Sep 22, 2024 at 11:29=E2=80= =AFAM Mark Johnston <<a href=3D"mailto:markj@freebsd.org">markj@freebsd.= org</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"marg= in:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e= x">On Fri, Sep 20, 2024 at 03:58:10PM +0000, Warner Losh wrote:<br> > The branch main has been updated by imp:<br> > <br> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D94567c8641e2= 35763b5b2926416d89d36654cde1" rel=3D"noreferrer" target=3D"_blank">https://= cgit.FreeBSD.org/src/commit/?id=3D94567c8641e235763b5b2926416d89d36654cde1<= /a><br> > <br> > commit 94567c8641e235763b5b2926416d89d36654cde1<br> > Author:=C2=A0 =C2=A0 =C2=A0Bram <<a href=3D"mailto:bram@cbbg.nl" ta= rget=3D"_blank">bram@cbbg.nl</a>><br> > AuthorDate: 2024-07-23 08:57:42 +0000<br> > Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> > CommitDate: 2024-09-20 15:06:26 +0000<br> > <br> >=C2=A0 =C2=A0 =C2=A0netstat: Resolve duplicate "dropped-packets&qu= ot; key from libxo output<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0The current libxo output uses the "dropped-pac= kets" key for both incoming and<br> >=C2=A0 =C2=A0 =C2=A0outgoing packets. This commit distinguishes between= the two by adding an "in"<br> >=C2=A0 =C2=A0 =C2=A0and "out" suffix. The original "drop= ped-packets" key is kept for backwards<br> >=C2=A0 =C2=A0 =C2=A0compatibility for now.<br> >=C2=A0 =C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0PR: 244589<br> >=C2=A0 =C2=A0 =C2=A0Reviewed by: imp, zlei<br> >=C2=A0 =C2=A0 =C2=A0Pull Request: <a href=3D"https://github.com/freebsd= /freebsd-src/pull/1331" rel=3D"noreferrer" target=3D"_blank">https://github= .com/freebsd/freebsd-src/pull/1331</a><br> > ---<br> >=C2=A0 usr.bin/netstat/if.c | 10 ++++++++--<br> >=C2=A0 1 file changed, 8 insertions(+), 2 deletions(-)<br> > <br> > diff --git a/usr.bin/netstat/if.c b/usr.bin/netstat/if.c<br> > index 172ea5324ccb..f0af785bce04 100644<br> > --- a/usr.bin/netstat/if.c<br> > +++ b/usr.bin/netstat/if.c<br> > @@ -501,8 +501,11 @@ intpr(void (*pfunc)(char *), int af)<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0IF= A_STAT(ipackets), link|network, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("= lu", nerr_len, "received-errors", IFA_STAT(ierrors),<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0li= nk, 1);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Below is kept for = backwards compatibility. Will be removed in the future. */<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("= lu", nerr_len, "dropped-packets", IFA_STAT(iqdrops),<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0li= nk, 1);<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("lu&qu= ot;, nerr_len, "dropped-packets-in", IFA_STAT(iqdrops),<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0link, 1= );<br> <br> This breaks plain netstat output, causing a bunch of network tests to<br> fail.=C2=A0 For instance, "netstat -Wf link -I re0" now prints an= extra<br> column.<br> <br> I think we should not bother with backwards compat unless libxo is<br> emitting structured output.=C2=A0 This can be implemented using the "e= "<br> format modifier described in the xo_format modified, but I'm not sure<b= r> how to plumb it through show_stat().<br> <br> Let's revert until this is fixed?<br></blockquote><div><br></div><div>W= orks for me. I hadn't noticed before the push. Thanks for bringing it u= p.</div><div><br></div><div>Warner</div><div>=C2=A0</div><blockquote class= =3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg= b(204,204,204);padding-left:1ex"> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (bflag)<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0show_stat("lu", nbyte_len, "received-bytes"= ;,<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0IFA_STAT(ibytes), link|network, 0);<br> > @@ -516,7 +519,7 @@ intpr(void (*pfunc)(char *), int af)<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("= NRSlu", nerr_len, "collisions", IFA_STAT(collisions),<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0li= nk, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (dflag)<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0show_stat("LSlu", nerr_len, "dropped-packets",<b= r> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0show_stat("LSlu", nerr_len, "dropped-packets-out"= ;,<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0IFA_STAT(oqdrops), link, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0xo_emit("\n= ");<br> >=C2=A0 <br> > @@ -705,8 +708,11 @@ loop:<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_ip - old->ift_i= p, 1, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("lu", 5, "received-= errors",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_ie - old->ift_i= e, 1, 1);<br> > +=C2=A0 =C2=A0 =C2=A0/* Below is kept for backwards compatibility. Wil= l be removed in the future. */<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("lu", 5, "dropped-p= ackets",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_id - old->ift_i= d, 1, 1);<br> > +=C2=A0 =C2=A0 =C2=A0show_stat("lu", 5, "dropped-packet= s-in",<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_id - old->ift_id, 1,= 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("lu", 10, "received= -bytes",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_ib - old->ift_i= b, 1, 0);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("lu", 10, "sent-pac= kets",<br> > @@ -718,7 +724,7 @@ loop:<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("NRSlu", 5, "collis= ions",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new->ift_co - old->ift_c= o, 1, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0if (dflag)<br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("LSlu&= quot;, 5, "dropped-packets",<br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0show_stat("LSlu&= quot;, 5, "dropped-packets-out",<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ne= w->ift_od - old->ift_od, 1, 1);<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0xo_close_instance("stats");<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0xo_emit("\n");<br> </blockquote></div></div> --00000000000076754d0622b521c7--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqAaAGc2ES8gtOX6Tx2WZsSthGdcSxUNtcZWKqYDwKzSQ>