Re: [PATCH (repost) net-next] sched: add extack for tfilter_notify

Jakub Kicinski
2022-11-02T23:41:13Z

On Wed, 2 Nov 2022 11:33:18 -0400 Jamal Hadi Salim wrote:
> > Sorry for the late response. I just came back form vacation. For this issue,
> > I saw netlink_dump_done() also put NLMSGERR_ATTR_MSG in NLMSG_DONE.
> > So why can't we do the same here?
> >
> > In https://www.kernel.org/doc/html//next/userspace-api/netlink/intro.html,
> > The "optionally extended ACK" in NLMSG_DONE is OK.
>
> Ok.
> [That seemd to be a nice doc - need to find time to look at it]

Thanks.

> > > Also - i guess it will depend on the underlying driver?
> > > This seems very related to a specific driver:
> > > "Warning: mlx5_core: matching on ct_state +new isn't supported."
> > > Debuggability is always great but so is backwards compat.
> > > What happens when you run old userspace tc? There are tons
> > > of punting systems that process these events out there and
> > > depend on the current event messages as is.
> >
> > I think old tc should just ignore this NLMSGERR_ATTR_MSG?
>
> Yes.
> So looks good to me then.
>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

Eish.

Hangbin, I'm still against this. Please go back to my suggestions /
questions. A tracepoint or an attribute should do. Multi-part messages
are very hard to map to normal programming constructs, and I don't
think there is any precedent for mutli-part notifications.