[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:51:42 UTC 2011
I forgot to add that I'm going to resend out the series, since your suggestions had a large ripple effect.
--Justin
On Nov 8, 2011, at 11:50 AM, Justin Pettit wrote:
> 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