[ovs-dev] [netlink v4 42/52] datapath: Convert flows to use Netlink framing.

[Jesse Gross]

On Thu, Jan 20, 2011 at 12:38 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Tue, Jan 18, 2011 at 05:54:54PM -0800, Jesse Gross wrote:
>> On Tue, Jan 11, 2011 at 9:49 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > diff --git a/datapath/flow.c b/datapath/flow.c
>> > index d4b7176..5d502ab 100644
>> > --- a/datapath/flow.c
>> > +++ b/datapath/flow.c
>> > -u32 flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb)
>> > +int flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb)
>> [...]
>> > ? ? ? ?nla_put_be16(skb, ODPKT_ETHERTYPE, swkey->dl_type);
>>
>> Should this be NLA_PUT_BE16?  Also, I don't think that anything uses
>> the non-macro versions of these functions that you added.  Should we
>> remove them now since they aren't really compat code?
>
> I'm willing, if you want to, I guess.

I'd like to remove them since it would have caught exactly this case
of accidentally using non-standard code.

>> > diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
>> > index 38a06ea..c610a3f 100644
>> > --- a/lib/dpif-linux.c
>> > +++ b/lib/dpif-linux.c
>> > +static int
>> > +dpif_linux_flow_from_ofpbuf(struct dpif_linux_flow *flow,
[...]
>> > + ? ? ? ?[ODPFT_ACTIONS] = { .type = NL_A_NESTED, .optional = true },
>> > + ? ? ? ?[ODPFT_STATS] = { .type = NL_A_UNSPEC,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ?.min_len = sizeof(struct odp_flow_stats),
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ?.max_len = sizeof(struct odp_flow_stats),
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ?.optional = true },
>> > + ? ? ? ?[ODPFT_TCP_FLAGS] = { .type = NL_A_U8, .optional = true },
>> > + ? ? ? ?[ODPFT_USED] = { .type = NL_A_U64, .optional = true },
>> > + ? ? ? ?[ODPFT_CLEAR] = { .type = NL_A_FLAG, .optional = true },
>>
>> What does clear mean in this context?
>
> Zero statistics.

Right.  What I meant was: what does it mean in the context of the
kernel passing it to userspace?