Re: Questions about the code review process in OpenBSD
Nov 5, 2022, 00:19 by ch...@nmedia.net: > i...@tutanota.com [i...@tutanota.com] wrote: > >> >> Is it a condition for code to go into the OpenBSD source tree (not >> talking about ports) that at least one other developer has reviewed the >> code? >> > > Yes > >> Is there a process in place to guarantee this? >> > > Yes, manual review all the way down. > > If someone commits without review from people who have experience in an area, > their work will be reversed. If they are really acting improperly, their > account will be removed. > Thank you very much for clarifying this. Kind regards.
Re: Questions about the code review process in OpenBSD
i...@tutanota.com [i...@tutanota.com] wrote: > > Is it a condition for code to go into the OpenBSD source tree (not > talking about ports) that at least one other developer has reviewed the > code? > Yes > Is there a process in place to guarantee this? > Yes, manual review all the way down. If someone commits without review from people who have experience in an area, their work will be reversed. If they are really acting improperly, their account will be removed.
Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips
On Fri, Nov 04, 2022 at 10:11:55AM -0500, John Browning wrote: > Hey, > It's: hw.vendor=LENOVO > hw.product=20Y4S1QE00 > hw.version=ThinkPad P1 Gen 4i > > That patch seems to be a better method. > > Thanks! thanks for the report, committed
Questions about the code review process in OpenBSD
I am trying to understand how the code review process is conducted in OpenBSD. I can see all the OK's in the commit log, but not every commit has the OK. On FreeBSD there where a serious problem with a developer who was hired to by Netgear to create a WireGuard VPN implementation as a kernel-mode solution and this was then contributed to FreeBSD. It was removed in the last minute. https://arstechnica.com/gadgets/2021/03/buffer-overruns-license-violations-and-bad-code-freebsd-13s-close-call/ Is it a condition for code to go into the OpenBSD source tree (not talking about ports) that at least one other developer has reviewed the code? Is there a process in place to guarantee this? If it's not a condition and anyone with commit access can commit freely, how do you prevent something like a committer going "rogue" and inserts a backdoor or creates another serious problem? Cheers.
pflow(4): make `so' dereference safe
Each pflow(4) interface has associated socket, referenced as sc->so. We set this socket in pflowioctl() which is called with both kernel and net locks held. In the pflow_output_process() task we do sc->so dereference, which is protected by kernel lock. But the sosend(), called deeper by pflow_output_process(), has sleep points, introduced by solock(). So this kernel lock protection doesn't work as expected, and concurrent pflowioctl() could override this sc->so. The diff below introduces per pflow(4) instance `sc_lock' rwlock(9) to protect sc->so. Since the solock() of udp(4) sockets uses netlock as backend, the `sc_lock' should be taken first. This expands a little netlock relocking within pflowioctl(). Also, pflow_sendout_mbuf() called by pflow_output_process(), now called without kernel lock held, so the mp safe counters_pkt(9) used instead of manual `if_opackets' increment. Since if_detach() does some ifnet destruction, now it can't be called before we finish pflow_output_process() task, otherwise we introduce use after free for interface counters. In other hand, we need to deny pflowioctl() to reschedule pflow_output_process() task. The `sc_dyind' flag introduced for that. Hrvoje, could you test this diff please? Index: sys/net/if_pflow.c === RCS file: /cvs/src/sys/net/if_pflow.c,v retrieving revision 1.96 diff -u -p -r1.96 if_pflow.c --- sys/net/if_pflow.c 12 Aug 2022 16:38:50 - 1.96 +++ sys/net/if_pflow.c 4 Nov 2022 18:23:27 - @@ -132,11 +132,11 @@ pflow_output_process(void *arg) struct mbuf *m; mq_delist(>sc_outputqueue, ); - KERNEL_LOCK(); + rw_enter_read(>sc_lock); while ((m = ml_dequeue()) != NULL) { pflow_sendout_mbuf(sc, m); } - KERNEL_UNLOCK(); + rw_exit_read(>sc_lock); } int @@ -146,6 +146,7 @@ pflow_clone_create(struct if_clone *ifc, struct pflow_softc *pflowif; pflowif = malloc(sizeof(*pflowif), M_DEVBUF, M_WAITOK|M_ZERO); + rw_init(>sc_lock, "pflowlk"); MGET(pflowif->send_nam, M_WAIT, MT_SONAME); pflowif->sc_version = PFLOW_PROTO_DEFAULT; @@ -254,6 +255,7 @@ pflow_clone_create(struct if_clone *ifc, mq_init(>sc_outputqueue, 8192, IPL_SOFTNET); pflow_setmtu(pflowif, ETHERMTU); pflow_init_timeouts(pflowif); + if_counters_alloc(ifp); if_attach(ifp); if_alloc_sadl(ifp); @@ -275,6 +277,7 @@ pflow_clone_destroy(struct ifnet *ifp) error = 0; NET_LOCK(); + sc->sc_dying = 1; SLIST_REMOVE(_list, sc, pflow_softc, sc_next); NET_UNLOCK(); @@ -475,10 +478,17 @@ pflowioctl(struct ifnet *ifp, u_long cmd struct pflowreq pflowr; int error; + if (sc->sc_dying) + return ENXIO; + switch (cmd) { case SIOCSIFADDR: case SIOCSIFDSTADDR: case SIOCSIFFLAGS: + /* XXXSMP: enforce lock order */ + NET_UNLOCK(); + rw_enter_read(>sc_lock); + NET_LOCK(); if ((ifp->if_flags & IFF_UP) && sc->so != NULL) { ifp->if_flags |= IFF_RUNNING; sc->sc_gcounter=pflowstats.pflow_flows; @@ -487,6 +497,7 @@ pflowioctl(struct ifnet *ifp, u_long cmd pflow_sendout_ipfix_tmpl(sc); } else ifp->if_flags &= ~IFF_RUNNING; + rw_exit_read(>sc_lock); break; case SIOCSIFMTU: if (ifr->ifr_mtu < PFLOW_MINMTU) @@ -523,10 +534,13 @@ pflowioctl(struct ifnet *ifp, u_long cmd /* XXXSMP breaks atomicity */ NET_UNLOCK(); + rw_enter_write(>sc_lock); error = pflow_set(sc, ); NET_LOCK(); - if (error != 0) + if (error != 0) { + rw_exit_write(>sc_lock); return (error); + } if ((ifp->if_flags & IFF_UP) && sc->so != NULL) { ifp->if_flags |= IFF_RUNNING; @@ -535,6 +549,7 @@ pflowioctl(struct ifnet *ifp, u_long cmd pflow_sendout_ipfix_tmpl(sc); } else ifp->if_flags &= ~IFF_RUNNING; + rw_exit_write(>sc_lock); break; @@ -1196,8 +1211,8 @@ pflow_sendout_ipfix_tmpl(struct pflow_so int pflow_sendout_mbuf(struct pflow_softc *sc, struct mbuf *m) { - sc->sc_if.if_opackets++; - sc->sc_if.if_obytes += m->m_pkthdr.len; + counters_pkt(sc->sc_if.if_counters, + ifc_opackets, ifc_obytes, m->m_pkthdr.len); if (sc->so == NULL) { m_freem(m); Index: sys/net/if_pflow.h === RCS file: /cvs/src/sys/net/if_pflow.h,v
Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips
On Fri, Nov 04, 2022 at 04:22:55PM +0100, Mark Kettenis wrote: > > Date: Sat, 5 Nov 2022 02:06:11 +1100 > > From: Jonathan Gray > > > > On Fri, Nov 04, 2022 at 09:10:52AM -0500, John Browning wrote: > > > Hi, > > > I noticed I did not have sound on my new thinkpad which has a newer > > > Intel 500 HDA chipset. > > > > > > bsd$ doas pcidump -vvv 0:31:3 > > > 0:31:3: Intel 500 Series HD Audio > > > 0x: Vendor ID: 8086, Product ID: 43c8 > > > 0x0004: Command: 0006, Status: 0010 > > > 0x0008:Class: 04 Multimedia, Subclass: 01 Audio, > > > Interface: 00, Revision: 11 > > > 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00, > > > Cache Line Size: 00 > > > 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000 > > > 0x0018: BAR empty () > > > 0x001c: BAR empty () > > > 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010 > > > 0x0028: Cardbus CIS: > > > 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4 > > > 0x0030: Expansion ROM Base Address: > > > 0x0038: > > > 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00 > > > 0x0050: Capability 0x01: Power Management > > > State: D0 > > > 0x0080: Capability 0x09: Vendor Specific > > > 0x0060: Capability 0x05: Message Signalled Interrupts (MSI) > > > Enabled: yes > > > > > > > > > Looking at src/sys/dev/pci/azalia.c I noticed making the change in the > > > below diff corrects the issue with a new kernel build. It seems the > > > pci subsystem match inclusion may not be needed, but hopefully someone > > > smarter than I can figure that out (or help me figure that out). > > > > When the subclass is not hd-audio, we match with azalia_pci_devices[]. > > > > Which model thinkpad is this? > > ok kettenis@ ok thfr@ (came up with the same diff based on a preceding discussion with John Browning on #openbsd-gaming IRC on libera.chat; also with input from brynet@) > > Index: sys/dev/pci/azalia.c > > === > > RCS file: /cvs/src/sys/dev/pci/azalia.c,v > > retrieving revision 1.280 > > diff -u -p -r1.280 azalia.c > > --- sys/dev/pci/azalia.c26 Oct 2022 20:19:08 - 1.280 > > +++ sys/dev/pci/azalia.c4 Nov 2022 14:10:59 - > > @@ -488,6 +488,7 @@ const struct pci_matchid azalia_pci_devi > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }, > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }, > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_LP_HDA }, > > + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_HDA }, > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_LP_HDA }, > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_600SERIES_LP_HDA }, > > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_GLK_HDA }, > > > > >
Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips
> Date: Sat, 5 Nov 2022 02:06:11 +1100 > From: Jonathan Gray > > On Fri, Nov 04, 2022 at 09:10:52AM -0500, John Browning wrote: > > Hi, > > I noticed I did not have sound on my new thinkpad which has a newer > > Intel 500 HDA chipset. > > > > bsd$ doas pcidump -vvv 0:31:3 > > 0:31:3: Intel 500 Series HD Audio > > 0x: Vendor ID: 8086, Product ID: 43c8 > > 0x0004: Command: 0006, Status: 0010 > > 0x0008:Class: 04 Multimedia, Subclass: 01 Audio, > > Interface: 00, Revision: 11 > > 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00, > > Cache Line Size: 00 > > 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000 > > 0x0018: BAR empty () > > 0x001c: BAR empty () > > 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010 > > 0x0028: Cardbus CIS: > > 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4 > > 0x0030: Expansion ROM Base Address: > > 0x0038: > > 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00 > > 0x0050: Capability 0x01: Power Management > > State: D0 > > 0x0080: Capability 0x09: Vendor Specific > > 0x0060: Capability 0x05: Message Signalled Interrupts (MSI) > > Enabled: yes > > > > > > Looking at src/sys/dev/pci/azalia.c I noticed making the change in the > > below diff corrects the issue with a new kernel build. It seems the > > pci subsystem match inclusion may not be needed, but hopefully someone > > smarter than I can figure that out (or help me figure that out). > > When the subclass is not hd-audio, we match with azalia_pci_devices[]. > > Which model thinkpad is this? ok kettenis@ > Index: sys/dev/pci/azalia.c > === > RCS file: /cvs/src/sys/dev/pci/azalia.c,v > retrieving revision 1.280 > diff -u -p -r1.280 azalia.c > --- sys/dev/pci/azalia.c 26 Oct 2022 20:19:08 - 1.280 > +++ sys/dev/pci/azalia.c 4 Nov 2022 14:10:59 - > @@ -488,6 +488,7 @@ const struct pci_matchid azalia_pci_devi > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_LP_HDA }, > + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_LP_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_600SERIES_LP_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_GLK_HDA }, > >
Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips
Hey, It's: hw.vendor=LENOVO hw.product=20Y4S1QE00 hw.version=ThinkPad P1 Gen 4i That patch seems to be a better method. Thanks! On Fri, Nov 4, 2022 at 10:06 AM Jonathan Gray wrote: > > On Fri, Nov 04, 2022 at 09:10:52AM -0500, John Browning wrote: > > Hi, > > I noticed I did not have sound on my new thinkpad which has a newer > > Intel 500 HDA chipset. > > > > bsd$ doas pcidump -vvv 0:31:3 > > 0:31:3: Intel 500 Series HD Audio > > 0x: Vendor ID: 8086, Product ID: 43c8 > > 0x0004: Command: 0006, Status: 0010 > > 0x0008:Class: 04 Multimedia, Subclass: 01 Audio, > > Interface: 00, Revision: 11 > > 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00, > > Cache Line Size: 00 > > 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000 > > 0x0018: BAR empty () > > 0x001c: BAR empty () > > 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010 > > 0x0028: Cardbus CIS: > > 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4 > > 0x0030: Expansion ROM Base Address: > > 0x0038: > > 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00 > > 0x0050: Capability 0x01: Power Management > > State: D0 > > 0x0080: Capability 0x09: Vendor Specific > > 0x0060: Capability 0x05: Message Signalled Interrupts (MSI) > > Enabled: yes > > > > > > Looking at src/sys/dev/pci/azalia.c I noticed making the change in the > > below diff corrects the issue with a new kernel build. It seems the > > pci subsystem match inclusion may not be needed, but hopefully someone > > smarter than I can figure that out (or help me figure that out). > > When the subclass is not hd-audio, we match with azalia_pci_devices[]. > > Which model thinkpad is this? > > Index: sys/dev/pci/azalia.c > === > RCS file: /cvs/src/sys/dev/pci/azalia.c,v > retrieving revision 1.280 > diff -u -p -r1.280 azalia.c > --- sys/dev/pci/azalia.c26 Oct 2022 20:19:08 - 1.280 > +++ sys/dev/pci/azalia.c4 Nov 2022 14:10:59 - > @@ -488,6 +488,7 @@ const struct pci_matchid azalia_pci_devi > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_LP_HDA }, > + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_LP_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_600SERIES_LP_HDA }, > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_GLK_HDA },
Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips
On Fri, Nov 04, 2022 at 09:10:52AM -0500, John Browning wrote: > Hi, > I noticed I did not have sound on my new thinkpad which has a newer > Intel 500 HDA chipset. > > bsd$ doas pcidump -vvv 0:31:3 > 0:31:3: Intel 500 Series HD Audio > 0x: Vendor ID: 8086, Product ID: 43c8 > 0x0004: Command: 0006, Status: 0010 > 0x0008:Class: 04 Multimedia, Subclass: 01 Audio, > Interface: 00, Revision: 11 > 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00, > Cache Line Size: 00 > 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000 > 0x0018: BAR empty () > 0x001c: BAR empty () > 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010 > 0x0028: Cardbus CIS: > 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4 > 0x0030: Expansion ROM Base Address: > 0x0038: > 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00 > 0x0050: Capability 0x01: Power Management > State: D0 > 0x0080: Capability 0x09: Vendor Specific > 0x0060: Capability 0x05: Message Signalled Interrupts (MSI) > Enabled: yes > > > Looking at src/sys/dev/pci/azalia.c I noticed making the change in the > below diff corrects the issue with a new kernel build. It seems the > pci subsystem match inclusion may not be needed, but hopefully someone > smarter than I can figure that out (or help me figure that out). When the subclass is not hd-audio, we match with azalia_pci_devices[]. Which model thinkpad is this? Index: sys/dev/pci/azalia.c === RCS file: /cvs/src/sys/dev/pci/azalia.c,v retrieving revision 1.280 diff -u -p -r1.280 azalia.c --- sys/dev/pci/azalia.c26 Oct 2022 20:19:08 - 1.280 +++ sys/dev/pci/azalia.c4 Nov 2022 14:10:59 - @@ -488,6 +488,7 @@ const struct pci_matchid azalia_pci_devi { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_LP_HDA }, + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_HDA }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_LP_HDA }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_600SERIES_LP_HDA }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_GLK_HDA },
special case mpe(4) in in6_ifattach()
So mpe(4) is a special device. It is a point-to-multipoint interface that does not do multicast. So setting IFF_MULTICAST on the interface is not correct but IPv6 depends on it because neighbor discovery. Now there is no neighbor discovery on mpe(4) the neighbors are handled via BGP. So lets adjust in6_ifattach() to succeed for mpe(4) interfaces and with that BGP IPv6 L3VPN should work. It may be an idea to move the IFF_MULTCAST check down into the ifp->if_type switch but right now this is good enough. I wonder if we have other interfaces that need similar treatment (e.g. mgre). -- :wq Claudio Index: netinet6/in6_ifattach.c === RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v retrieving revision 1.119 diff -u -p -r1.119 in6_ifattach.c --- netinet6/in6_ifattach.c 8 Sep 2022 10:22:07 - 1.119 +++ netinet6/in6_ifattach.c 4 Nov 2022 12:52:36 - @@ -373,7 +373,8 @@ in6_ifattach(struct ifnet *ifp) if (ifp->if_mtu < IPV6_MMTU) return (EINVAL); - if ((ifp->if_flags & IFF_MULTICAST) == 0) + /* ND needs multicast but mpe(4) does neither multicast nor ND */ + if ((ifp->if_flags & IFF_MULTICAST) == 0 && ifp->if_type != IFT_MPLS) return (EINVAL); /* @@ -389,10 +390,14 @@ in6_ifattach(struct ifnet *ifp) return (error); } - /* Interfaces that rely on strong a priori cryptographic binding of -* IP addresses are incompatible with automatically assigned llv6. */ + /* +* Some interface don't need an automatically assigned link-local +* address either because it think it is special (wireguard) or +* because there is no need and no neighbor discovery (mpe). +*/ switch (ifp->if_type) { case IFT_WIREGUARD: + case IFT_MPLS: return (0); }
azalia module fix for 7.2-stable and some Intel 500 HDA Chips
Hi, I noticed I did not have sound on my new thinkpad which has a newer Intel 500 HDA chipset. bsd$ doas pcidump -vvv 0:31:3 0:31:3: Intel 500 Series HD Audio 0x: Vendor ID: 8086, Product ID: 43c8 0x0004: Command: 0006, Status: 0010 0x0008:Class: 04 Multimedia, Subclass: 01 Audio, Interface: 00, Revision: 11 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00, Cache Line Size: 00 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000 0x0018: BAR empty () 0x001c: BAR empty () 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010 0x0028: Cardbus CIS: 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4 0x0030: Expansion ROM Base Address: 0x0038: 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00 0x0050: Capability 0x01: Power Management State: D0 0x0080: Capability 0x09: Vendor Specific 0x0060: Capability 0x05: Message Signalled Interrupts (MSI) Enabled: yes Looking at src/sys/dev/pci/azalia.c I noticed making the change in the below diff corrects the issue with a new kernel build. It seems the pci subsystem match inclusion may not be needed, but hopefully someone smarter than I can figure that out (or help me figure that out). Index: sys/dev/pci/azalia.c === RCS file: /cvs/src/sys/dev/pci/azalia.c,v retrieving revision 1.276 diff -u -p -u -p -r1.276 azalia.c --- sys/dev/pci/azalia.c8 Sep 2022 01:28:46 -1.276 +++ sys/dev/pci/azalia.c4 Nov 2022 14:02:31 - @@ -511,8 +511,7 @@ azalia_pci_match(struct device *parent, struct pci_attach_args *pa; pa = aux; -if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MULTIMEDIA -&& PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_HDAUDIO) +if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MULTIMEDIA) return 1; return pci_matchbyid((struct pci_attach_args *)aux, azalia_pci_devices, nitems(azalia_pci_devices)); I hope this helps some folks! I apologize that I am using 7.2-stable and not -current. Thanks, John Browning
Re: rpki-client: missing initializer in output.c
On Fri, Nov 04, 2022 at 01:50:11PM +0100, Theo Buehler wrote: > Doesn't really matter, but it looks odd and -Wmissing-field-initializers > flags this. > > Index: output.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v > retrieving revision 1.27 > diff -u -p -r1.27 output.c > --- output.c 30 Aug 2022 18:56:49 - 1.27 > +++ output.c 4 Nov 2022 12:03:01 - > @@ -73,7 +73,7 @@ static const struct outputs { > { FORMAT_BIRD, "bird", output_bird2 }, > { FORMAT_CSV, "csv", output_csv }, > { FORMAT_JSON, "json", output_json }, > - { 0, NULL } > + { 0, NULL, NULL } > }; > > static FILE *output_createtmp(char *); > OK, an alternative is to use the c99 { 0 } instead. -- :wq Claudio
rpki-client: missing initializer in output.c
Doesn't really matter, but it looks odd and -Wmissing-field-initializers flags this. Index: output.c === RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v retrieving revision 1.27 diff -u -p -r1.27 output.c --- output.c30 Aug 2022 18:56:49 - 1.27 +++ output.c4 Nov 2022 12:03:01 - @@ -73,7 +73,7 @@ static const struct outputs { { FORMAT_BIRD, "bird", output_bird2 }, { FORMAT_CSV, "csv", output_csv }, { FORMAT_JSON, "json", output_json }, - { 0, NULL } + { 0, NULL, NULL } }; static FILE*output_createtmp(char *);
Re: bgpctl show mpls label in fib output
On Fri, Nov 04, 2022 at 09:12:13AM +0100, Theo Buehler wrote: > On Thu, Nov 03, 2022 at 03:26:35PM +0100, Claudio Jeker wrote: > > Noticed while figuring out the kroute bug with MPLS. > > I think it would be nice to know the MPLS label of a fib MPLS route. > > > > bgpctl show fib table 13 > > flags: B = BGP, C = Connected, S = Static > >N = BGP Nexthop reachable via this route > >r = reject route, b = blackhole route > > > > flags prio destination gateway > > C1 127.0.0.1/32 link#129 > > B 48 192.168.44.0/24 10.12.57.2 mpls 44 > > C1 192.168.237.242/32 link#128 > > > > Not sure if the keyword should be "mpls" or "label". > > I think something containing mpls is clearer. Did you deliberately use > mpls in output.c and mplslabel in output_json.c? Yes. The JSON label can be explicit while for the text output I prefer something short to not overflow the line. > ok > > > -- > > :wq Claudio > > > > Index: bgpctl.h > > === > > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.h,v > > retrieving revision 1.17 > > diff -u -p -r1.17 bgpctl.h > > --- bgpctl.h17 Oct 2022 12:01:19 - 1.17 > > +++ bgpctl.h3 Nov 2022 14:21:14 - > > @@ -58,3 +58,5 @@ const char*fmt_community(uint16_t, uint > > const char *fmt_large_community(uint32_t, uint32_t, uint32_t); > > const char *fmt_ext_community(uint8_t *); > > const char *fmt_set_type(struct ctl_show_set *); > > + > > +#define MPLS_LABEL_OFFSET 12 > > Index: output.c > > === > > RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v > > retrieving revision 1.30 > > diff -u -p -r1.30 output.c > > --- output.c17 Oct 2022 12:01:19 - 1.30 > > +++ output.c3 Nov 2022 14:21:33 - > > @@ -477,6 +477,8 @@ show_fib(struct kroute_full *kf) > > printf("link#%u", kf->ifindex); > > else > > printf("%s", log_addr(>nexthop)); > > + if (kf->flags & F_MPLS) > > + printf(" mpls %d", ntohl(kf->mplslabel) >> MPLS_LABEL_OFFSET); > > printf("\n"); > > } > > > > Index: output_json.c > > === > > RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v > > retrieving revision 1.24 > > diff -u -p -r1.24 output_json.c > > --- output_json.c 17 Oct 2022 12:01:19 - 1.24 > > +++ output_json.c 3 Nov 2022 14:22:48 - > > @@ -385,6 +385,9 @@ json_fib(struct kroute_full *kf) > > else > > json_do_printf("nexthop", "%s", log_addr(>nexthop)); > > > > + if (kf->flags & F_CONNECTED) > > + json_do_printf("mplslabel", "%d", > > + ntohl(kf->mplslabel) >> MPLS_LABEL_OFFSET); > > json_do_end(); > > } > > > > > -- :wq Claudio
Re: bgpctl show mpls label in fib output
On Thu, Nov 03, 2022 at 03:26:35PM +0100, Claudio Jeker wrote: > Noticed while figuring out the kroute bug with MPLS. > I think it would be nice to know the MPLS label of a fib MPLS route. > > bgpctl show fib table 13 > flags: B = BGP, C = Connected, S = Static >N = BGP Nexthop reachable via this route >r = reject route, b = blackhole route > > flags prio destination gateway > C1 127.0.0.1/32 link#129 > B 48 192.168.44.0/24 10.12.57.2 mpls 44 > C1 192.168.237.242/32 link#128 > > Not sure if the keyword should be "mpls" or "label". I think something containing mpls is clearer. Did you deliberately use mpls in output.c and mplslabel in output_json.c? ok > -- > :wq Claudio > > Index: bgpctl.h > === > RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.h,v > retrieving revision 1.17 > diff -u -p -r1.17 bgpctl.h > --- bgpctl.h 17 Oct 2022 12:01:19 - 1.17 > +++ bgpctl.h 3 Nov 2022 14:21:14 - > @@ -58,3 +58,5 @@ const char *fmt_community(uint16_t, uint > const char *fmt_large_community(uint32_t, uint32_t, uint32_t); > const char *fmt_ext_community(uint8_t *); > const char *fmt_set_type(struct ctl_show_set *); > + > +#define MPLS_LABEL_OFFSET 12 > Index: output.c > === > RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v > retrieving revision 1.30 > diff -u -p -r1.30 output.c > --- output.c 17 Oct 2022 12:01:19 - 1.30 > +++ output.c 3 Nov 2022 14:21:33 - > @@ -477,6 +477,8 @@ show_fib(struct kroute_full *kf) > printf("link#%u", kf->ifindex); > else > printf("%s", log_addr(>nexthop)); > + if (kf->flags & F_MPLS) > + printf(" mpls %d", ntohl(kf->mplslabel) >> MPLS_LABEL_OFFSET); > printf("\n"); > } > > Index: output_json.c > === > RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v > retrieving revision 1.24 > diff -u -p -r1.24 output_json.c > --- output_json.c 17 Oct 2022 12:01:19 - 1.24 > +++ output_json.c 3 Nov 2022 14:22:48 - > @@ -385,6 +385,9 @@ json_fib(struct kroute_full *kf) > else > json_do_printf("nexthop", "%s", log_addr(>nexthop)); > > + if (kf->flags & F_CONNECTED) > + json_do_printf("mplslabel", "%d", > + ntohl(kf->mplslabel) >> MPLS_LABEL_OFFSET); > json_do_end(); > } > >