| Message ID | 1442983222-42750-1-git-send-email-roopa@cumulusnetworks.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Roopa Prabhu <roopa@cumulusnetworks.com> Date: Tue, 22 Sep 2015 21:40:22 -0700 > From: Wilson Kok <wkok@cumulusnetworks.com> > > dump_rules returns skb length and not error. > But when family == AF_UNSPEC, the caller of dump_rules > assumes that it returns an error. Hence, when family == AF_UNSPEC, > we continue trying to dump on -EMSGSIZE errors resulting in > incorrect dump idx carried between skbs belonging to the same dump. > This results in fib rule dump always only dumping rules that fit > into the first skb. > > This patch fixes dump_rules to return error so that we exit correctly > and idx is correctly maintained between skbs that are part of the > same dump. > > Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com> > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Applied and queued up for -stable, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 22, 2015 at 9:40 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > + err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid, > + cb->nlh->nlmsg_seq, RTM_NEWRULE, > + NLM_F_MULTI, ops); > + if (err) FWIW I believe this breaks pre-4.0 stable kernels (unfortunately it just showed up in 3.10.90). In kernels without 053c095a82cf ("netlink: make nlmsg_end() and genlmsg_end() void") then fib_nl_fill_rule() returns a positive value (skb->len) on success, so we break out of the loop here immediately. Symptom is "ip rule show" loops forever printing the first rule. After I finish testing a fix (as trivial as changing to "if (err < 0)") here, I'll send it to -stable guys. - R. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/2/15, 10:18 AM, Roland Dreier wrote: > On Tue, Sep 22, 2015 at 9:40 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >> + err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid, >> + cb->nlh->nlmsg_seq, RTM_NEWRULE, >> + NLM_F_MULTI, ops); >> + if (err) > FWIW I believe this breaks pre-4.0 stable kernels (unfortunately it > just showed up in 3.10.90). In kernels without 053c095a82cf > ("netlink: make nlmsg_end() and genlmsg_end() void") then > fib_nl_fill_rule() returns a positive value (skb->len) on success, so > we break out of the loop here immediately. Symptom is "ip rule show" > loops forever printing the first rule. > > After I finish testing a fix (as trivial as changing to "if (err < > 0)") here, I'll send it to -stable guys. > Thanks for catching this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index bf77e36..365de66 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -631,15 +631,17 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb, { int idx = 0; struct fib_rule *rule; + int err = 0; rcu_read_lock(); list_for_each_entry_rcu(rule, &ops->rules_list, list) { if (idx < cb->args[1]) goto skip; - if (fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid, - cb->nlh->nlmsg_seq, RTM_NEWRULE, - NLM_F_MULTI, ops) < 0) + err = fib_nl_fill_rule(skb, rule, NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, RTM_NEWRULE, + NLM_F_MULTI, ops); + if (err) break; skip: idx++; @@ -648,7 +650,7 @@ skip: cb->args[1] = idx; rules_ops_put(ops); - return skb->len; + return err; } static int fib_nl_dumprule(struct sk_buff *skb, struct netlink_callback *cb) @@ -664,7 +666,9 @@ static int fib_nl_dumprule(struct sk_buff *skb, struct netlink_callback *cb) if (ops == NULL) return -EAFNOSUPPORT; - return dump_rules(skb, cb, ops); + dump_rules(skb, cb, ops); + + return skb->len; } rcu_read_lock();