[altq 1236] Re: ALTQ conditioners on OpenBSD bridges

[Jason Wright <jason@thought.net>]

On Sat, Dec 08, 2001 at 12:54:56AM +0100, Rémi Guyomarch wrote:
> Ok, I think I have a working code.
> 
> This little patch relative to OpenBSD 3.0 implements ALTQ conditioning
> on bridged interfaces. Note that since all the hard work has been done
> in the function sending IP frames to PF for filtering, I simply put
> the ALTQ call just after the PF call.
> 
This is pretty cool, and I'm amazed at how small the diff is =)  I must
say that I don't know much about ALTQs internals, but from what I see
this diff appears to be correct.

> With this feature you can implement a transparent diffedge shaping and
> filtering device, which is quite nice :) (no need to renumber or
> adjust routes on your border routers)
> 
> Btw, I'm not by far an experienced kernel hacker. In fact it's the
> first time I do kernel programming so I may have done something
> terrible ;)
> 
Appears you've done well enough =)  Comments below in the code...

> --- if_bridge.c 2001/08/21 15:18:20     1.76
> +++ if_bridge.c 2001/12/07 22:58:14
> @@ -1043,7 +1043,7 @@
>                 m_freem(m);
>                 return;
>         }
> -#if NPF > 0
> +#if (NPF > 0) || defined(ALTQ)
>         m = bridge_filter(sc, BRIDGE_IN, src_if, &eh, m);
>         if (m == NULL)
>                 return;
> @@ -1911,7 +1911,7 @@
>         return (0);
>  }
>  
> -#if NPF > 0
> +#if (NPF > 0) || defined(ALTQ)
>  /*
>   * Filter IP packets by peeking into the ethernet frame.  This violates
>   * the ISO model, but allows us to act as a IP filter at the data link
> @@ -1926,9 +1926,6 @@
>         struct ether_header *eh;
>         struct mbuf *m;
>  {
> -#if NPF == 0
> -       return (m);
> -#else
>         struct llc llc;
>         int hassnap = 0;
>         struct ip *ip;
> @@ -2000,9 +1997,10 @@
>  
>         /* Finally, we get to filter the packet! */
>         m->m_pkthdr.rcvif = ifp;
> +#if NPF > 0
>         if (pf_test(dir, ifp, &m) != PF_PASS)
>                 goto dropit;
> -
> +#endif
>         /* Rebuild the IP header */
>         if (m->m_len < hlen && ((m = m_pullup(m, hlen)) == NULL))
>                 return (NULL);
> @@ -2015,6 +2013,16 @@
>         ip->ip_sum = 0;
>         ip->ip_sum = in_cksum(m, hlen);
>  
> +#ifdef ALTQ
> +       if (altq_input != NULL && dir == BRIDGE_IN) {
> +               if ((*altq_input)(m, AF_INET) == 0)
> +                       /* packet is dropped by traffic conditioner */
> +                       goto dropit;
> +               ip->ip_sum = 0;
> +               ip->ip_sum = in_cksum(m, hlen);
> +       }
> +#endif /* ALTQ */
> +

Ok, my question here is: is the header checksum recomputation necessary?
Also, altq treats the input mbuf as readonly correct? (No destructive
operations, including m_pullup(), are used).

--Jason L. Wright