[ovs-dev] [PATCH 1/2] datapath: Set the correct bits for OFPAT_SET_NW_TOS action.

[Ben Pfaff]

On Fri, Feb 12, 2010 at 02:37:42PM -0800, Justin Pettit wrote:
> On Feb 11, 2010, at 3:22 PM, Ben Pfaff wrote:
> 
> > @@ -265,10 +266,10 @@ static struct sk_buff *set_nw_tos(struct sk_buff *skb,
> > 		struct iphdr *nh = ip_hdr(skb);
> > 		u8 *f = &nh->tos;
> > 		u8 old = *f;
> > +		u8 new;
> > 
> > -		/* We only set the lower 6 bits. */
> > -		u8 new = (a->nw_tos & 0x3f) | (nh->tos & 0xc0);
> > -
> > +		/* Set the DSCP bits and preserve the ECN bits. */
> > +		new = (a->nw_tos & ~INET_ECN_MASK) | (nh->tos & INET_ECN_MASK);
> 
> Is there a reason to not use (a->nw_tos & IP_DSCP_MASK) instead of
> negating INET_ECN_MASK? It seems a bit clearer to me and more
> consistent with code dpif-netdev.c.

IP_DSCP_MASK isn't defined in any kernel header (we define it in
lib/packets.h for our userspace code), but <net/inet_ecn.h> defines
INET_ECN_MASK and uses ~INET_ECN_MASK to get the DSCP mask, so I
followed that style for the kernel code.

> Otherwise looks good.

Thanks, I pushed it to "openflow-1.0".