| Message ID | 20240301012845.2951053-4-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netlink: handle EMSGSIZE errors in the core | expand |
On Fri, Mar 1, 2024 at 2:31 AM Jakub Kicinski <kuba@kernel.org> wrote: > > Make sure ctrl_fill_info() returns sensible error codes and > propagate them out to netlink core. Let netlink core decide > when to return skb->len and when to treat the exit as an > error. Netlink core does better job at it, if we always > return skb->len the core doesn't know when we're done > dumping and NLMSG_DONE ends up in a separate read(). > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: jiri@resnulli.us > --- > net/netlink/genetlink.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c > index 50ec599a5cff..70379ecfb6ed 100644 > --- a/net/netlink/genetlink.c > +++ b/net/netlink/genetlink.c > @@ -1232,7 +1232,7 @@ static int ctrl_fill_info(const struct genl_family *family, u32 portid, u32 seq, > > hdr = genlmsg_put(skb, portid, seq, &genl_ctrl, flags, cmd); > if (hdr == NULL) > - return -1; > + return -EMSGSIZE; > > if (nla_put_string(skb, CTRL_ATTR_FAMILY_NAME, family->name) || > nla_put_u16(skb, CTRL_ATTR_FAMILY_ID, family->id) || > @@ -1355,6 +1355,7 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) > struct net *net = sock_net(skb->sk); > int fams_to_skip = cb->args[0]; > unsigned int id; > + int err; Don't we have to initialize err to 0 ? Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks ! > > idr_for_each_entry(&genl_fam_idr, rt, id) { > if (!rt->netnsok && !net_eq(net, &init_net)) > @@ -1363,16 +1364,17 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) > if (n++ < fams_to_skip) > continue; > > - if (ctrl_fill_info(rt, NETLINK_CB(cb->skb).portid, > - cb->nlh->nlmsg_seq, NLM_F_MULTI, > - skb, CTRL_CMD_NEWFAMILY) < 0) { > + err = ctrl_fill_info(rt, NETLINK_CB(cb->skb).portid, > + cb->nlh->nlmsg_seq, NLM_F_MULTI, > + skb, CTRL_CMD_NEWFAMILY); > + if (err) { > n--; > break; > } > } > > cb->args[0] = n; > - return skb->len; > + return err; > } > > static struct sk_buff *ctrl_build_family_msg(const struct genl_family *family, > -- > 2.43.2 >
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 50ec599a5cff..70379ecfb6ed 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -1232,7 +1232,7 @@ static int ctrl_fill_info(const struct genl_family *family, u32 portid, u32 seq, hdr = genlmsg_put(skb, portid, seq, &genl_ctrl, flags, cmd); if (hdr == NULL) - return -1; + return -EMSGSIZE; if (nla_put_string(skb, CTRL_ATTR_FAMILY_NAME, family->name) || nla_put_u16(skb, CTRL_ATTR_FAMILY_ID, family->id) || @@ -1355,6 +1355,7 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) struct net *net = sock_net(skb->sk); int fams_to_skip = cb->args[0]; unsigned int id; + int err; idr_for_each_entry(&genl_fam_idr, rt, id) { if (!rt->netnsok && !net_eq(net, &init_net)) @@ -1363,16 +1364,17 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) if (n++ < fams_to_skip) continue; - if (ctrl_fill_info(rt, NETLINK_CB(cb->skb).portid, - cb->nlh->nlmsg_seq, NLM_F_MULTI, - skb, CTRL_CMD_NEWFAMILY) < 0) { + err = ctrl_fill_info(rt, NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, NLM_F_MULTI, + skb, CTRL_CMD_NEWFAMILY); + if (err) { n--; break; } } cb->args[0] = n; - return skb->len; + return err; } static struct sk_buff *ctrl_build_family_msg(const struct genl_family *family,
Make sure ctrl_fill_info() returns sensible error codes and propagate them out to netlink core. Let netlink core decide when to return skb->len and when to treat the exit as an error. Netlink core does better job at it, if we always return skb->len the core doesn't know when we're done dumping and NLMSG_DONE ends up in a separate read(). Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: jiri@resnulli.us --- net/netlink/genetlink.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)