On Tue, May 10, 2022 at 12:21:02AM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> On Mon, May 09, 2022 at 06:01:07PM +0300, Barbaros Bilek wrote:
> > Hello,
> >
> > I was using veb (veb+vlan+ixl) interfaces quite stable since 6.9.
> > My system ran as a firewall under OpenBSD 6.9 and 7.0 quite stable.
> > Also I've used 7.1 for a limited time and there were no crash.
> > After OpenBSD' NET_TASKQ upgrade to 4 it crashed after 5 days.
> > Here crash report and dmesg:
> >
> > ether_input(ffff80000520e000,fffffd8053616700) at ether_input+0x3ad
> > vport_if_enqueue(ffff80000520e000,fffffd8053616700) at vport_if_enqueue+0x19
> > veb_port_input(ffff8000051c3800,fffffd806064c200,ffffffffffff,ffff800002066600)
> > at veb_port_input+0x4d2
> > ether_input(ffff8000051c3800,fffffd806064c200) at ether_input+0x100
> > end trace frame: 0xffff800025575290, count: 0
> > ddb{1}> show panic
> >
> > *cpu1: kernel diagnostic assertion "curcpu()->ci_schedstate.spc_smrdepth ==
> > 0" f
> > ailed: file "/usr/src/sys/kern/subr_xxx.c", line 163
> >
> > ddb{1}> trace
> >
> > db_enter() at db_enter+0x10
> >
> > panic(ffffffff81f22e39) at panic+0xbf
> >
> > __assert(ffffffff81f96c9d,ffffffff81f85ebc,a3,ffffffff81fd252f) at
> > __assert+0x2
> >
> > 5
> >
>
> diff below attempts to fix this particular panic triggered by veb_span()
> function. This is fairly simple/straightforward change:
>
> we grab references to veb ports inside SMR_READ_ section.
>
> we keep those references in single linked list
>
> as soon as we leave SMR_READ_ section we process the list:
> dispatch packets
>
> drop references to port
>
> The change may uncover similar panics in other veb/bridge area.
>
> diff applies to current
>
> thanks for testing and reporting back.
Can we limit the number of span ports per bridge to a small number so that
the instead of a heap object for the SLIST a simple stack array of
MAX_SPAN_PORTS pointers could be used?
Who needs more than a handfull of spanports per veb?
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/net/if_veb.c b/sys/net/if_veb.c
> index 2976cc200f1..a02dbac887f 100644
> --- a/sys/net/if_veb.c
> +++ b/sys/net/if_veb.c
> @@ -159,6 +159,11 @@ struct veb_softc {
> struct veb_ports sc_spans;
> };
>
> +struct veb_span_port {
> + SLIST_ENTRY(veb_span_port) sp_entry;
> + struct veb_port *sp_port;
> +};
> +
> #define DPRINTF(_sc, fmt...) do { \
> if (ISSET((_sc)->sc_if.if_flags, IFF_DEBUG)) \
> printf(fmt); \
> @@ -225,6 +230,7 @@ static struct if_clone veb_cloner =
> IF_CLONE_INITIALIZER("veb", veb_clone_create, veb_clone_destroy);
>
> static struct pool veb_rule_pool;
> +static struct pool span_port_pool;
>
> static int vport_clone_create(struct if_clone *, int);
> static int vport_clone_destroy(struct ifnet *);
> @@ -266,6 +272,11 @@ veb_clone_create(struct if_clone *ifc, int unit)
> 0, IPL_SOFTNET, 0, "vebrpl", NULL);
> }
>
> + if (span_port_pool.pr_size == 0) {
> + pool_init(&span_port_pool, sizeof(struct veb_span_port),
> + 0, IPL_SOFTNET, 0, "vebspl", NULL);
> + }
> +
> sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO|M_CANFAIL);
> if (sc == NULL)
> return (ENOMEM);
> @@ -352,22 +363,38 @@ veb_span(struct veb_softc *sc, struct mbuf *m0)
> struct veb_port *p;
> struct ifnet *ifp0;
> struct mbuf *m;
> + struct veb_span_port *sp;
> + SLIST_HEAD(, veb_span_port) span_list;
>
> + SLIST_INIT(&span_list)
> smr_read_enter();
> SMR_TAILQ_FOREACH(p, &sc->sc_spans.l_list, p_entry) {
> ifp0 = p->p_ifp0;
> if (!ISSET(ifp0->if_flags, IFF_RUNNING))
> continue;
>
> - m = m_dup_pkt(m0, max_linkhdr + ETHER_ALIGN, M_NOWAIT);
> - if (m == NULL) {
> - /* XXX count error */
> - continue;
> - }
> + sp = pool_get(&span_port_pool, PR_NOWAIT);
> + if (sp == NULL)
> + continue; /* XXX count error */
>
> - if_enqueue(ifp0, m); /* XXX count error */
> + veb_eb_brport_take(p);
> + sp->sp_port = p;
> + SLIST_INSERT_HEAD(&span_list, sp, sp_entry);
> }
> smr_read_leave();
> +
> + while (!SLIST_EMPTY(&span_list)) {
> + sp = SLIST_FIRST(&span_list);
> + SLIST_REMOVE_HEAD(&span_list, sp_entry);
> +
> + m = m_dup_pkt(m0, max_linkhdr + ETHER_ALIGN, M_NOWAIT);
> + if (m != NULL)
> + if_enqueue(sp->sp_port->p_ifp0, m);
> + /* XXX count error */
> +
> + veb_eb_brport_rele(sp->sp_port);
> + pool_put(&span_port_pool, sp);
> + }
> }
>
> static int
>
--
:wq Claudio
