On 15/05/15(Fri) 17:34, mxb wrote:
> Diff is applied. So far no problems.
> Unfortunately I can’t test this fully - no vlans on my side.
Thanks for testing. A "no regression" report is always welcome.
There's some more issues with bridge+vlan but jasper@ also confirmed
this diff improve the situation.
Can I have oks?
> > On 15 maj 2015, at 13:14, Martin Pieuchot <m...@openbsd.org> wrote:
> >
> > I have one setup with multiple interfaces in a bridge and on some of
> > these interfaces some vlan(4)s. But there's currently a bug that
> > prevent us to send (receive is fine) VLAN packets in such config.
> > Diff below fixes that.
> >
> > The problem is that vlan_output() does not pass its parent interface
> > to ether_output(). That's a mis-design that should be fixed later.
> > The reason for not passing the parent interface is that we want to
> > tcpdump(8) packets on vlan interfaces and the easiest hack^Wsolution
> > was to add a bpf handler in vlan_start()*.
> >
> > Since my vlans are not part of the bridge, the check below is never
> > true and my packets never go through the bridge. By moving this
> > check to if_output() we kill two birds with one diff. First of
> > all we fix this vlan bug and secondly we simplify ether_output()
> > which in turn will allow us to fix all pseudo-interface *output()
> > functions.
> >
> > One of the goals of if_output() is to move all bpf handlers instead
> > of having them in multiple if_start(). Of course, this will also
> > help us removing the various "#if PSEUDODRIVER" from our stack...
> >
> > Ok?
> >
> > *: Note that for the exact same reason we cannot tcpdump output
> > packets on a carp(4) interface, this will be fixed at the same
> > time in upcoming diffs.
> >
> >
> > Index: net/if_ethersubr.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> > retrieving revision 1.198
> > diff -u -p -r1.198 if_ethersubr.c
> > --- net/if_ethersubr.c 15 May 2015 10:15:13 -0000 1.198
> > +++ net/if_ethersubr.c 15 May 2015 10:58:37 -0000
> > @@ -363,47 +363,6 @@ ether_output(struct ifnet *ifp0, struct
> > if (ether_addheader(&m, ifp, etype, esrc, edst) == -1)
> > senderr(ENOBUFS);
> >
> > -#if NBRIDGE > 0
> > - /*
> > - * Interfaces that are bridgeports need special handling for output.
> > - */
> > - if (ifp->if_bridgeport) {
> > - struct m_tag *mtag;
> > -
> > - /*
> > - * Check if this packet has already been sent out through
> > - * this bridgeport, in which case we simply send it out
> > - * without further bridge processing.
> > - */
> > - for (mtag = m_tag_find(m, PACKET_TAG_BRIDGE, NULL); mtag;
> > - mtag = m_tag_find(m, PACKET_TAG_BRIDGE, mtag)) {
> > -#ifdef DEBUG
> > - /* Check that the information is there */
> > - if (mtag->m_tag_len != sizeof(caddr_t)) {
> > - error = EINVAL;
> > - goto bad;
> > - }
> > -#endif
> > - if (!memcmp(&ifp->if_bridgeport, mtag + 1,
> > - sizeof(caddr_t)))
> > - break;
> > - }
> > - if (mtag == NULL) {
> > - /* Attach a tag so we can detect loops */
> > - mtag = m_tag_get(PACKET_TAG_BRIDGE, sizeof(caddr_t),
> > - M_NOWAIT);
> > - if (mtag == NULL) {
> > - error = ENOBUFS;
> > - goto bad;
> > - }
> > - memcpy(mtag + 1, &ifp->if_bridgeport, sizeof(caddr_t));
> > - m_tag_prepend(m, mtag);
> > - error = bridge_output(ifp, m, NULL, NULL);
> > - return (error);
> > - }
> > - }
> > -#endif
> > -
> > len = m->m_pkthdr.len;
> >
> > error = if_output(ifp, m);
> > Index: net/if.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.331
> > diff -u -p -r1.331 if.c
> > --- net/if.c 15 May 2015 10:15:13 -0000 1.331
> > +++ net/if.c 15 May 2015 10:58:37 -0000
> > @@ -450,6 +450,40 @@ if_output(struct ifnet *ifp, struct mbuf
> > length = m->m_pkthdr.len;
> > mflags = m->m_flags;
> >
> > +#if NBRIDGE > 0
> > + /*
> > + * Interfaces that are bridgeports need special handling for output.
> > + */
> > + if (ifp->if_bridgeport) {
> > + struct m_tag *mtag;
> > +
> > + /*
> > + * Check if this packet has already been sent out through
> > + * this bridgeport, in which case we simply send it out
> > + * without further bridge processing.
> > + */
> > + for (mtag = m_tag_find(m, PACKET_TAG_BRIDGE, NULL); mtag;
> > + mtag = m_tag_find(m, PACKET_TAG_BRIDGE, mtag)) {
> > + if (!memcmp(&ifp->if_bridgeport, mtag + 1,
> > + sizeof(caddr_t)))
> > + break;
> > + }
> > + if (mtag == NULL) {
> > + /* Attach a tag so we can detect loops */
> > + mtag = m_tag_get(PACKET_TAG_BRIDGE, sizeof(caddr_t),
> > + M_NOWAIT);
> > + if (mtag == NULL) {
> > + m_freem(m);
> > + return (ENOBUFS);
> > + }
> > + memcpy(mtag + 1, &ifp->if_bridgeport, sizeof(caddr_t));
> > + m_tag_prepend(m, mtag);
> > + error = bridge_output(ifp, m, NULL, NULL);
> > + return (error);
> > + }
> > + }
> > +#endif
> > +
> > s = splnet();
> >
> > /*
> >
>
>
