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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221102163646.131a3910@kernel.org>
Date:   Wed, 2 Nov 2022 16:36:46 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        Hangbin Liu <liuhangbin@...il.com>
Cc:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH (repost) net-next] sched: add extack for tfilter_notify

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@...atatu.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.