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

Jesse Gross jesse at nicira.com
Wed Jul 31 21:08:10 UTC 2013


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