On Thu, Nov 03, 2011 at 01:21:25PM +0100, Camiel Dobbelaar wrote:
> No one interested in this one? I have another bridge speedup diff after
> this.
>
Ugh, forgot to answer on that one. I'm against overusing the M_PROTO1
flag. Keep in mind that M_PROTO1 is just a flag whereas the mbuf_tag is a
pointer to the ifp. So M_PROTO1 does not scale if there are multiple
bridges involved in forwarding a packet. M_PROTO1 is only OK to use when
the flag is cleared when leaving the protocol. E.g. it can be used when a
hint needs to be passed accross an ifq.
--
:wq Claudio
> On Fri, 28 Oct 2011, Camiel Dobbelaar wrote:
>
> > M_PROTO1 is used by if_bridge on the input path. On the output path it's
> > used now only by if_bridge for if_gif. I think we can use it generically
> > to mark packets as "processed by bridge" in the output path.
> >
> > The diff simplifies things and avoids mtag checking and allocation so is
> > more efficient too.
> >
> > The old code checks if a packet has passed the _same_ bridge already, but
> > as an interface can only be a member of one bridge I think the flag is
> > sufficient.
> >
> > It looks like the only other user of M_PROTO1 is netbt/hci_link.c, but
> > that can be fixed if the diff is acceptable otherwise.
> >
> > Tested lightly in a bridge/gif setup, but could use some more testing.
> > (especially with ipsec in the mix too)
> >
> >
> > Index: if_bridge.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if_bridge.c,v
> > retrieving revision 1.193
> > diff -u -p -r1.193 if_bridge.c
> > --- if_bridge.c 4 Jul 2011 06:54:49 -0000 1.193
> > +++ if_bridge.c 28 Oct 2011 17:55:04 -0000
> > @@ -2813,9 +2813,8 @@ bridge_ifenqueue(struct bridge_softc *sc
> > #if NGIF > 0
> > /* Packet needs etherip encapsulation. */
> > if (ifp->if_type == IFT_GIF) {
> > - m->m_flags |= M_PROTO1;
> > -
> > /* Count packets input into the gif from outside */
> > + /* XXX do this in if_gif? */
> > ifp->if_ipackets++;
> > ifp->if_ibytes += m->m_pkthdr.len;
> > }
> > @@ -2844,6 +2843,7 @@ bridge_ifenqueue(struct bridge_softc *sc
> > }
> > #endif
> > len = m->m_pkthdr.len;
> > + m->m_flags |= M_PROTO1;
> > mflags = m->m_flags;
> > IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
> > if (error) {
> > Index: if_ethersubr.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> > retrieving revision 1.151
> > diff -u -p -r1.151 if_ethersubr.c
> > --- if_ethersubr.c 9 Jul 2011 00:47:18 -0000 1.151
> > +++ if_ethersubr.c 28 Oct 2011 17:55:04 -0000
> > @@ -382,40 +382,8 @@ ether_output(ifp0, m0, dst, rt0)
> > * Interfaces that are bridge members need special handling
> > * for output.
> > */
> > - if (ifp->if_bridge) {
> > - struct m_tag *mtag;
> > -
> > - /*
> > - * Check if this packet has already been sent out through
> > - * this bridge, 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 (!bcmp(&ifp->if_bridge, 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;
> > - }
> > - bcopy(&ifp->if_bridge, mtag + 1, sizeof(caddr_t));
> > - m_tag_prepend(m, mtag);
> > - error = bridge_output(ifp, m, NULL, NULL);
> > - return (error);
> > - }
> > - }
> > + if (ifp->if_bridge && !(m->m_flags & M_PROTO1))
> > + return (bridge_output(ifp, m, NULL, NULL));
> > #endif
> > mflags = m->m_flags;
> > len = m->m_pkthdr.len;
