[ovs-dev] [PATCH 2/2] odp-util: always export priority and skb_mark in netlink

Jesse Gross jesse at nicira.com
Wed Jul 31 22:08:57 UTC 2013


(0/0) is also an unmasked key.

This is exactly my point. There should not be a sliver of a doubt in
anyone's mind as to whether 0 is a special value. It's not.

On Wed, Jul 31, 2013 at 3:05 PM, Andy Zhou <azhou at nicira.com> wrote:
> We need to print (5/0) in order to how the unmasked key.
>
>
> On Wed, Jul 31, 2013 at 2:08 PM, Jesse Gross <jesse at nicira.com> wrote:
>>
>> This is debugging output, so it should print exactly what is received.
>> If there are more compact ways of representing it then that is fine as
>> long as it retains the same meaning.
>>
>> In addition, zero is no longer a special value so a value of (5/0)
>> should be treated the same as (0/0) but this isn't doing that.
>>
>> On Wed, Jul 31, 2013 at 1:45 PM, Andy Zhou <azhou at nicira.com> wrote:
>> > Yes, it is possible with more hacking on the test scripts. Omitting them
>> > would make the output easier to read in general -- having more (0/0)
>> > does
>> > not add more information. In general we are moving in a direction of not
>> > output unnecessary information, removing output mask of 255.255.255.255,
>> > for
>> > example.
>> >
>> >
>> >
>> > On Wed, Jul 31, 2013 at 1:27 PM, Jesse Gross <jesse at nicira.com> wrote:
>> >>
>> >> Can't you just add skb_mark to the input?
>> >>
>> >> On Wed, Jul 31, 2013 at 12:39 PM, Andy Zhou <azhou at nicira.com> wrote:
>> >> > This helps to keep the test easy: Input is the same as output.
>> >> >
>> >> >
>> >> > On Wed, Jul 31, 2013 at 11:39 AM, Jesse Gross <jesse at nicira.com>
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Jul 30, 2013 at 7:49 PM, Andy Zhou <azhou at nicira.com> wrote:
>> >> >> > Handling of missing attributes in netlink can be tricky and turns
>> >> >> > out
>> >> >> > to be error prone. The value (savings in netlink bandwidth) does
>> >> >> > not
>> >> >> > seem to be significant enough to justify allowing them. This patch
>> >> >> > series make both kernel and userspace always export priority and
>> >> >> > skb_mark attribute. There will be follow on patches in the
>> >> >> > direction of making all attributes explicit.
>> >> >> >
>> >> >> > Signed-off-by: Andy Zhou <azhou at nicira.com>
>> >> >> > ---
>> >> >> >  lib/odp-util.c        |   62
>> >> >> > +++++++++++++++++++++++++++++++++++++++----------
>> >> >> >  tests/ofproto-dpif.at |   18 +++++++-------
>> >> >> >  2 files changed, 59 insertions(+), 21 deletions(-)
>> >> >> >
>> >> >> > diff --git a/lib/odp-util.c b/lib/odp-util.c
>> >> >> > index 3c3063d..1f7db2f 100644
>> >> >> > --- a/lib/odp-util.c
>> >> >> > +++ b/lib/odp-util.c
>> >> >> > @@ -926,6 +926,42 @@ odp_mask_attr_is_exact(const struct nlattr
>> >> >> > *ma)
>> >> >> >      return is_exact;
>> >> >> >  }
>> >> >> >
>> >> >> > +static bool
>> >> >> > +ommit_format(enum ovs_key_attr attr, const struct nlattr *a,
>> >> >> > +             const struct nlattr *ma)
>> >> >> > +{
>> >> >> > +    switch (attr) {
>> >> >> > +        case OVS_KEY_ATTR_PRIORITY:
>> >> >> > +        case OVS_KEY_ATTR_SKB_MARK:
>> >> >> > +            if (!nl_attr_get_u32(a)) {
>> >> >> > +                if ((!ma) || !nl_attr_get_u32(ma)) {
>> >> >> > +                    return true;  /* Omit output 0 (no mask) or
>> >> >> > 0/0
>> >> >> > */
>> >> >> > +                }
>> >> >> > +            }
>> >> >> > +            break;
>> >> >> > +        case OVS_KEY_ATTR_UNSPEC:
>> >> >> > +        case OVS_KEY_ATTR_ENCAP:
>> >> >> > +        case OVS_KEY_ATTR_TUNNEL:
>> >> >> > +        case OVS_KEY_ATTR_IN_PORT:
>> >> >> > +        case OVS_KEY_ATTR_ETHERNET:
>> >> >> > +        case OVS_KEY_ATTR_VLAN:
>> >> >> > +        case OVS_KEY_ATTR_ETHERTYPE:
>> >> >> > +        case OVS_KEY_ATTR_IPV4:
>> >> >> > +        case OVS_KEY_ATTR_IPV6:
>> >> >> > +        case OVS_KEY_ATTR_TCP:
>> >> >> > +        case OVS_KEY_ATTR_UDP:
>> >> >> > +        case OVS_KEY_ATTR_ICMP:
>> >> >> > +        case OVS_KEY_ATTR_ICMPV6:
>> >> >> > +        case OVS_KEY_ATTR_ARP:
>> >> >> > +        case OVS_KEY_ATTR_ND:
>> >> >> > +        case OVS_KEY_ATTR_MPLS:
>> >> >> > +        case __OVS_KEY_ATTR_MAX:
>> >> >> > +        default:
>> >> >> > +            break;
>> >> >> > +    }
>> >> >> > +
>> >> >> > +    return false;
>> >> >> > +}
>> >> >>
>> >> >> Does it actually make sense to omit printing of these fields still?
>> >> >> After all, we print fully wildcarded other fields and this is really
>> >> >> debugging output.
>> >> >
>> >> >
>> >
>> >
>
>



More information about the dev mailing list