[ovs-dev] [#8024 5/7] Don't overload IP TOS with the frag matching bits.

Justin Pettit jpettit at nicira.com
Tue Nov 8 19:50:42 UTC 2011


On Nov 7, 2011, at 12:34 PM, Ben Pfaff wrote:

> kernel
> ------
> 
> Do you plan to use the top 6 bits of ip.frag for something later on?
> If not, I think we can drop all the bitwise manipulation that assumes
> that something might be in those bits, get rid of OVS_FRAG_TYPE_MASK,
> etc.

My thinking was that currently it only holds fragment flags, but we may want to use it for other flags later.  After discussing off-line with you and Jesse, none of us had a strong preference for leaving the masks or taking them out.  I've gone ahead and removed them, since they'd probably need some revisiting even if we did add new flags.

> #defining OVS_FRAG_TYPE_MASK as INET_ECN_MASK makes a lot less sense
> now, anyway.

I had meant to update that, but missed it.

> Dropping parse_tos_frag() dropped validation of the ipv4_tos and
> ipv4_frag members from flow_from_nlattrs().  Probably it's good to
> still validate them.

Okay, I've put in new validation checks.

> user
> ----
> 
> The tos field isn't bitwise maskable so we could use a FWW_* bit for
> it.  (It might not be worth the churn though.)

I agree.  I've got some cleanups of other things that I'd like address after this series, so I'll do that separately.

> Like the kernel, the frag field uses all the bits so it's probably
> worthwhile to drop most of the bitwise operations there too.

Yep.  Updated.

> In ofputil_normalize_rule(), I think I'd just rename MAY_TOS_FRAG to
> MAY_IPVx (or similar) instead of breaking it apart, since they always
> go together.

Yeah, I was thinking along those lines.  I've gone ahead and done that. 

--Justin





More information about the dev mailing list