| Message ID | 20240502113748.1622637-2-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 136c2a9a2a8760d8dae83ae7c882c50be02bdb63 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | rtnetlink: rtnl_stats_dump() changes | expand |
On 5/2/24 5:37 AM, Eric Dumazet wrote: > By returning 0 (or an error) instead of skb->len, > we allow NLMSG_DONE to be appended to the current > skb at the end of a dump, saving a couple of recvmsg() > system calls. any concern that a patch similar to: https://lore.kernel.org/netdev/20240411180202.399246-1-kuba@kernel.org/ will be needed again here? > > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > net/core/rtnetlink.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 283e42f48af68504af193ed5763d4e0fcd667d99..88980c8bcf334079e2d19cbcfb3f10fc05e3c19b 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -6024,7 +6024,7 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb) > cb->args[1] = idx; > cb->args[0] = h; > > - return skb->len; > + return err; > } > > void rtnl_offload_xstats_notify(struct net_device *dev) Reviewed-by: David Ahern <dsahern@kernel.org>
On Thu, May 2, 2024 at 5:59 PM David Ahern <dsahern@kernel.org> wrote: > > On 5/2/24 5:37 AM, Eric Dumazet wrote: > > By returning 0 (or an error) instead of skb->len, > > we allow NLMSG_DONE to be appended to the current > > skb at the end of a dump, saving a couple of recvmsg() > > system calls. > > any concern that a patch similar to: > https://lore.kernel.org/netdev/20240411180202.399246-1-kuba@kernel.org/ > will be needed again here? This has been discussed, Jakub answer was : https://lore.kernel.org/netdev/20240411115748.05faa636@kernel.org/ So the plan is to change functions until a regression is reported.
On 5/2/24 10:03 AM, Eric Dumazet wrote: > On Thu, May 2, 2024 at 5:59 PM David Ahern <dsahern@kernel.org> wrote: >> >> On 5/2/24 5:37 AM, Eric Dumazet wrote: >>> By returning 0 (or an error) instead of skb->len, >>> we allow NLMSG_DONE to be appended to the current >>> skb at the end of a dump, saving a couple of recvmsg() >>> system calls. >> >> any concern that a patch similar to: >> https://lore.kernel.org/netdev/20240411180202.399246-1-kuba@kernel.org/ >> will be needed again here? > > This has been discussed, Jakub answer was : > > https://lore.kernel.org/netdev/20240411115748.05faa636@kernel.org/ > > So the plan is to change functions until a regression is reported. > As I commented in the past, it is more user friendly to add such comments to a commit message so that when a regression occurs and a bisect is done, the user hitting the regression sees the problem with an obvious resolution.
On Mon, May 6, 2024 at 6:23 PM David Ahern <dsahern@kernel.org> wrote: > > On 5/2/24 10:03 AM, Eric Dumazet wrote: > > On Thu, May 2, 2024 at 5:59 PM David Ahern <dsahern@kernel.org> wrote: > >> > >> On 5/2/24 5:37 AM, Eric Dumazet wrote: > >>> By returning 0 (or an error) instead of skb->len, > >>> we allow NLMSG_DONE to be appended to the current > >>> skb at the end of a dump, saving a couple of recvmsg() > >>> system calls. > >> > >> any concern that a patch similar to: > >> https://lore.kernel.org/netdev/20240411180202.399246-1-kuba@kernel.org/ > >> will be needed again here? > > > > This has been discussed, Jakub answer was : > > > > https://lore.kernel.org/netdev/20240411115748.05faa636@kernel.org/ > > > > So the plan is to change functions until a regression is reported. > > > > As I commented in the past, it is more user friendly to add such > comments to a commit message so that when a regression occurs and a > bisect is done, the user hitting the regression sees the problem with an > obvious resolution. This commit has a single line being changed. Whoever does a bisection will report, and the resolution is trivial. I do not think we want to copy/paste a full page of all links to all relevant commits.
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 283e42f48af68504af193ed5763d4e0fcd667d99..88980c8bcf334079e2d19cbcfb3f10fc05e3c19b 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -6024,7 +6024,7 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb) cb->args[1] = idx; cb->args[0] = h; - return skb->len; + return err; } void rtnl_offload_xstats_notify(struct net_device *dev)
By returning 0 (or an error) instead of skb->len, we allow NLMSG_DONE to be appended to the current skb at the end of a dump, saving a couple of recvmsg() system calls. Signed-off-by: Eric Dumazet <edumazet@google.com> --- net/core/rtnetlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)