Date: Sun, 22 Sep 2024 17:29:15 +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: <CANCZdfrDq28V0caVeUSwmLzNWCcTwK7zkPdMZS%2Bz611XnNncWw@mail.gmail.com> In-Reply-To: <ZvA1CjfQ0HN69UrE@nuc> References: <202409201558.48KFwAEN048834@gitrepo.freebsd.org> <Zu_xk5GoyC2ykk7Q@nuc> <CANCZdfqAaAGc2ES8gtOX6Tx2WZsSthGdcSxUNtcZWKqYDwKzSQ@mail.gmail.com> <ZvA1CjfQ0HN69UrE@nuc>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Sun, Sep 22, 2024 at 4:17 PM Mark Johnston <markj@freebsd.org> wrote: > On Sun, Sep 22, 2024 at 02:19:54PM +0100, Warner Losh wrote: > > On Sun, Sep 22, 2024 at 11:29 AM 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=94567c8641e235763b5b2926416d89d36654cde1 > > > > > > > > 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. > > Thanks, the tests seem to be fixed now. > Thanks for the quick shout out it was a problem... [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Sep 22, 2024 at 4:17 PM Mark Johnston <<a href="mailto:markj@freebsd.org">markj@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, Sep 22, 2024 at 02:19:54PM +0100, Warner Losh wrote:<br> > On Sun, Sep 22, 2024 at 11:29 AM Mark Johnston <<a href="mailto:markj@freebsd.org" target="_blank">markj@freebsd.org</a>> wrote:<br> > <br> > > 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:<br> > > <a href="https://cgit.FreeBSD.org/src/commit/?id=94567c8641e235763b5b2926416d89d36654cde1" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=94567c8641e235763b5b2926416d89d36654cde1</a><br>; > > ><br> > > > commit 94567c8641e235763b5b2926416d89d36654cde1<br> > > > Author: Bram <<a href="mailto:bram@cbbg.nl" target="_blank">bram@cbbg.nl</a>><br> > > > AuthorDate: 2024-07-23 08:57:42 +0000<br> > > > Commit: Warner Losh <imp@FreeBSD.org><br> > > > CommitDate: 2024-09-20 15:06:26 +0000<br> > > ><br> > > > netstat: Resolve duplicate "dropped-packets" key from libxo output<br> > > ><br> > > > The current libxo output uses the "dropped-packets" key for both<br> > > incoming and<br> > > > outgoing packets. This commit distinguishes between the two by<br> > > adding an "in"<br> > > > and "out" suffix. The original "dropped-packets" key is kept for<br> > > backwards<br> > > > compatibility for now.<br> > > ><br> > > > PR: 244589<br> > > > Reviewed by: imp, zlei<br> > > > Pull Request: <a href="https://github.com/freebsd/freebsd-src/pull/1331" rel="noreferrer" target="_blank">https://github.com/freebsd/freebsd-src/pull/1331</a><br>; > > > ---<br> > > > usr.bin/netstat/if.c | 10 ++++++++--<br> > > > 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> > > > IFA_STAT(ipackets), link|network, 1);<br> > > > show_stat("lu", nerr_len, "received-errors",<br> > > IFA_STAT(ierrors),<br> > > > link, 1);<br> > > > + /* Below is kept for backwards compatibility. Will be<br> > > removed in the future. */<br> > > > show_stat("lu", nerr_len, "dropped-packets",<br> > > IFA_STAT(iqdrops),<br> > > > link, 1);<br> > > > + show_stat("lu", nerr_len, "dropped-packets-in",<br> > > IFA_STAT(iqdrops),<br> > > > + link, 1);<br> > ><br> > > This breaks plain netstat output, causing a bunch of network tests to<br> > > fail. 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. This can be implemented using the "e"<br> > > format modifier described in the xo_format modified, but I'm not sure<br> > > how to plumb it through show_stat().<br> > ><br> > > Let's revert until this is fixed?<br> > ><br> > <br> > Works for me. I hadn't noticed before the push. Thanks for bringing it up.<br> <br> Thanks, the tests seem to be fixed now.<br></blockquote><div><br></div><div>Thanks for the quick shout out it was a problem... </div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrDq28V0caVeUSwmLzNWCcTwK7zkPdMZS%2Bz611XnNncWw>