| Message ID | 4a951891dbdb4812406e7dbf9af9f61d65b7353c.1585217878.git.pabeni@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [net-next,v2] Squash-to: "mptcp: add netlink-based PM" | expand |
Hi Paolo, On 26/03/2020 11:19, Paolo Abeni wrote: > Let user space set a single limit at once and drop bogus > NL extack error message. Thank you for the patch! > > v1 -> v2: > - do not include dumb dbg messages > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > net/mptcp/pm_netlink.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 743c3c58f826..caad73dfda64 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -702,10 +702,8 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit) > { > struct nlattr *attr = info->attrs[id]; > > - if (!attr) { > - GENL_SET_ERR_MSG(info, "missing announce accept limit"); > - return -EINVAL; > - } > + if (!attr) > + return 0; I guess we don't need to return an error if no attributes are given for both RCV_ADD_ADDRS and SUBFLOWS, right? > > *limit = nla_get_u32(attr); > if (*limit > MPTCP_PM_ADDR_MAX) { Here below, the message is "announce accept limit greater than maximum". Should we keep only "limit greater than maximum"? > @@ -723,10 +721,12 @@ mptcp_nl_cmd_set_limits(struct sk_buff *skb, struct genl_info *info) > unsigned int rcv_addrs, subflows; > int ret; > > + rcv_addrs = pernet->add_addr_accept_max; I guess that's fine to always call the two WRITE_ONCE() here below even if it is not needed, right? Cheers, Matt
On Thu, 2020-03-26 at 11:34 +0100, Matthieu Baerts wrote: > Hi Paolo, > > On 26/03/2020 11:19, Paolo Abeni wrote: > > Let user space set a single limit at once and drop bogus > > NL extack error message. > > Thank you for the patch! > > > v1 -> v2: > > - do not include dumb dbg messages > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > net/mptcp/pm_netlink.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index 743c3c58f826..caad73dfda64 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > @@ -702,10 +702,8 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit) > > { > > struct nlattr *attr = info->attrs[id]; > > > > - if (!attr) { > > - GENL_SET_ERR_MSG(info, "missing announce accept limit"); > > - return -EINVAL; > > - } > > + if (!attr) > > + return 0; > > I guess we don't need to return an error if no attributes are given for > both RCV_ADD_ADDRS and SUBFLOWS, right? yep > > > > *limit = nla_get_u32(attr); > > if (*limit > MPTCP_PM_ADDR_MAX) { > > Here below, the message is "announce accept limit greater than maximum". > Should we keep only "limit greater than maximum"? agreed > > > @@ -723,10 +721,12 @@ mptcp_nl_cmd_set_limits(struct sk_buff *skb, struct genl_info *info) > > unsigned int rcv_addrs, subflows; > > int ret; > > > > + rcv_addrs = pernet->add_addr_accept_max; > > I guess that's fine to always call the two WRITE_ONCE() here below even > if it is not needed, right? I see no issue with that, but looking again at this, I guess it's better wrap everthing with spin lock protection. Additionally I need to relax the GENL_ADMIN_PERM on get commands. Thanks for the feedback, v3 is coming ;) /P
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 743c3c58f826..caad73dfda64 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -702,10 +702,8 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit) { struct nlattr *attr = info->attrs[id]; - if (!attr) { - GENL_SET_ERR_MSG(info, "missing announce accept limit"); - return -EINVAL; - } + if (!attr) + return 0; *limit = nla_get_u32(attr); if (*limit > MPTCP_PM_ADDR_MAX) { @@ -723,10 +721,12 @@ mptcp_nl_cmd_set_limits(struct sk_buff *skb, struct genl_info *info) unsigned int rcv_addrs, subflows; int ret; + rcv_addrs = pernet->add_addr_accept_max; ret = parse_limit(info, MPTCP_PM_ATTR_RCV_ADD_ADDRS, &rcv_addrs); if (ret) return ret; + subflows = pernet->subflows_max; ret = parse_limit(info, MPTCP_PM_ATTR_SUBFLOWS, &subflows); if (ret) return ret;
Let user space set a single limit at once and drop bogus NL extack error message. v1 -> v2: - do not include dumb dbg messages Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- net/mptcp/pm_netlink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)