Re: [PATCH 1/5] netlink: extended ACK reporting

On 18 April 2017 at 02:41, Johannes Berg wrote: > On Thu, 2017-04-13 at 14:44 -0700, Joe Stringer wrote > (something that never made it to the list, due to HTML formatting) >> >> I think that OVS was doing some more elaborate validation than most >> users, so over time

Re: [PATCH 1/5] netlink: extended ACK reporting

On Thu, 2017-04-13 at 14:44 -0700, Joe Stringer wrote (something that never made it to the list, due to HTML formatting) > > I think that OVS was doing some more elaborate validation than most > users, so over time we picked up a bunch of extra parsing code that > layers on top of nla_parse(). I

Re: [PATCH 1/5] netlink: extended ACK reporting

On Thu, 2017-04-13 at 16:05 +0200, Nicolas Dichtel wrote: > Sure. It was just to mention that attribute 0 exists somewhere. > The other 0 attribute is OVS_TUNNEL_KEY_ATTR_ID. That looks like some really awkward hand-grown parsing - with all these "struct ovs_len_tbl" looking almost like a

Re: [PATCH 1/5] netlink: extended ACK reporting

Le 13/04/2017 à 15:29, Johannes Berg a écrit : > On Thu, 2017-04-13 at 15:27 +0200, Nicolas Dichtel wrote: >> >>> Yes, some - very few - families still insist on using attribute 0, >>> perhaps parsing by hand or so. Like you say though, the entire >>> infrastructure makes that hard and

Re: [PATCH 1/5] netlink: extended ACK reporting

On Thu, 2017-04-13 at 15:27 +0200, Nicolas Dichtel wrote: > > > Yes, some - very few - families still insist on using attribute 0, > > perhaps parsing by hand or so. Like you say though, the entire > > infrastructure makes that hard and undesirable, so I don't really > > see > > why we need to

Re: [PATCH 1/5] netlink: extended ACK reporting

Le 10/04/2017 à 08:18, Johannes Berg a écrit : > >> perhaps I misunderstand something, but nla_parse suggests attribute >> type can not be 0: > [...] > > Yes, some - very few - families still insist on using attribute 0, > perhaps parsing by hand or so. Like you say though, the entire >

Re: [PATCH 1/5] netlink: extended ACK reporting

> perhaps I misunderstand something, but nla_parse suggests attribute > type can not be 0: [...] Yes, some - very few - families still insist on using attribute 0, perhaps parsing by hand or so. Like you say though, the entire infrastructure makes that hard and undesirable, so I don't really see

Re: [PATCH 1/5] netlink: extended ACK reporting

On 4/8/17 2:40 PM, Jiri Pirko wrote: > Sat, Apr 08, 2017 at 08:37:01PM CEST, johan...@sipsolutions.net wrote: >> On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote: >>> nla_total_size(sizeof(u32)); + if (extack && + (extack->missing_attr || extack-

Re: [PATCH 1/5] netlink: extended ACK reporting

On 4/8/17 1:48 PM, Johannes Berg wrote: > diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h > index f3946a27bd07..d1564557d645 100644 > --- a/include/uapi/linux/netlink.h > +++ b/include/uapi/linux/netlink.h > @@ -101,6 +101,29 @@ struct nlmsghdr { > struct nlmsgerr { >

Re: [PATCH 1/5] netlink: extended ACK reporting

On Sat, 2017-04-08 at 20:40 +0200, Jiri Pirko wrote: > > I think I'll leave it like this - if anyone really wants to say > > "attribute 0 is missing" then we can add a flag later... The UAPI > > does > > take this into account by not including the attribute at all if the > > data is invalid, so 0

Re: [PATCH 1/5] netlink: extended ACK reporting

Sat, Apr 08, 2017 at 08:37:01PM CEST, johan...@sipsolutions.net wrote: >On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote: >> nla_total_size(sizeof(u32)); >> > + if (extack && >> > + (extack->missing_attr || extack- >> > >bad_attr)) >> >> Attr could be 0,

Re: [PATCH 1/5] netlink: extended ACK reporting

On Sat, 2017-04-08 at 14:36 -0400, David Ahern wrote: > > I think v3 is in your future ... > > /home/dsa/kernel-4.git/include/linux/netlink.h:78:12: error: > ‘NETLINK_MAX_COOKIE_LEN’ undeclared here (not in a function) >   u8 cookie[NETLINK_MAX_COOKIE_LEN]; > ^ > > it's defined in

Re: [PATCH 1/5] netlink: extended ACK reporting

On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote: > nla_total_size(sizeof(u32)); > > + if (extack && > > + (extack->missing_attr || extack- > > >bad_attr)) > > Attr could be 0, right? I know that on the most of the places 0 is > UNSPEC, but I'm pretty

Re: [PATCH 1/5] netlink: extended ACK reporting

On 4/8/17 1:48 PM, Johannes Berg wrote: > diff --git a/include/linux/netlink.h b/include/linux/netlink.h > index da14ab61f363..47562e940e9c 100644 > --- a/include/linux/netlink.h > +++ b/include/linux/netlink.h > @@ -62,11 +62,41 @@ netlink_kernel_create(struct net *net, int unit, struct >

Re: [PATCH 1/5] netlink: extended ACK reporting

Sat, Apr 08, 2017 at 07:48:56PM CEST, johan...@sipsolutions.net wrote: >From: Johannes Berg > >Add the base infrastructure and UAPI for netlink >extended ACK reporting. All "manual" calls to >netlink_ack() pass NULL for now and thus don't >get extended ACK reporting. Why

Re: [PATCH 1/5] netlink: extended ACK reporting

On 4/8/17 1:48 PM, Johannes Berg wrote: > @@ -2267,21 +2284,37 @@ int __netlink_dump_start(struct sock *ssk, struct > sk_buff *skb, > } > EXPORT_SYMBOL(__netlink_dump_start); > > -void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err) > +void netlink_ack(struct sk_buff