On Wed, May 11, 2022 at 09:58:09AM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> >
> > 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?
>
> I just to make sure I follow your idea...
>
> >
> > > --------8<---------------8<---------------8<------------------8<--------
> > > diff --git a/sys/net/if_veb.c b/sys/net/if_veb.c
> > > index 2976cc200f1..a02dbac887f 100644
> </snip>
> > > + struct veb_span_port *sp;
> > > + SLIST_HEAD(, veb_span_port) span_list;
>
> so the span_list, will be turned to array of references, right?
>
> > >
> > > + 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);
>
> here we do instead of SLIST_INSERT_HEAD() something like:
> span_list[i++] = p;
>
> > > }
> > > smr_read_leave();
> > > +
> > > + while (!SLIST_EMPTY(&span_list)) {
>
> and here instead of walking list we just walk through array.
Yes, this is my basic idea.
> is that somewhat correct? If so than I need some 'qualified' suggestion
> for upper limit of span ports. I'm afraid my real life experience
> with network switches is zero, so I don't feel like a right person.
I would suggest an upper limit of 8. For example it seems Cisco only
supports 2 span sessions so 8 should be plenty.
> another idea I'm toying with is to add a 'service task' to every switch
> so the underlying `vport_if_enqueue()` will put packet to queue and
> let task to dispatch to ifp associated with vport. Once packet
> will be dispatched the task will drop the reference to port.
Need to look more into this but in general I dislike adding more queues
and tasks. Such a setup would just synchronize everything back to a single
thread.
--
:wq Claudio
