ipv4: Stop taking ECN bits into account in fib4-rules

Use the new dscp_t type to replace the tos field of struct fib4_rule,
so that fib4-rules consistently ignore ECN bits.

Before this patch, fib4-rules did accept rules with the high order ECN
bit set (but not the low order one). Also, it relied on its callers
masking the ECN bits of ->flowi4_tos to prevent those from influencing
the result. This was brittle and a few call paths still do the lookup
without masking the ECN bits first.

After this patch fib4-rules only compare the DSCP bits. ECN can't
influence the result anymore, even if the caller didn't mask these
bits. Also, fib4-rules now must have both ECN bits cleared or they will
be rejected.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
Acked-by: David Ahern <dsahern@kernel.org>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index e0b6c8b..117c485 100644 (file)
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -23,6 +23,7 @@
 #include <linux/list.h>
 #include <linux/rcupdate.h>
 #include <linux/export.h>
+#include <net/inet_dscp.h>
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/tcp.h>
@@ -35,7 +36,7 @@ struct fib4_rule {
        struct fib_rule         common;
        u8                      dst_len;
        u8                      src_len;
-       u8                      tos;
+       dscp_t                  dscp;
        __be32                  src;
        __be32                  srcmask;
        __be32                  dst;
@@ -49,7 +50,7 @@ static bool fib4_rule_matchall(const struct fib_rule *rule)
 {
        struct fib4_rule *r = container_of(rule, struct fib4_rule, common);
 
-       if (r->dst_len || r->src_len || r->tos)
+       if (r->dst_len || r->src_len || r->dscp)
                return false;
        return fib_rule_matchall(rule);
 }
@@ -185,7 +186,7 @@ INDIRECT_CALLABLE_SCOPE int fib4_rule_match(struct fib_rule *rule,
            ((daddr ^ r->dst) & r->dstmask))
                return 0;
 
-       if (r->tos && (r->tos != fl4->flowi4_tos))
+       if (r->dscp && r->dscp != inet_dsfield_to_dscp(fl4->flowi4_tos))
                return 0;
 
        if (rule->ip_proto && (rule->ip_proto != fl4->flowi4_proto))
@@ -225,10 +226,12 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
        int err = -EINVAL;
        struct fib4_rule *rule4 = (struct fib4_rule *) rule;
 
-       if (frh->tos & ~IPTOS_TOS_MASK) {
-               NL_SET_ERR_MSG(extack, "Invalid tos");
+       if (!inet_validate_dscp(frh->tos)) {
+               NL_SET_ERR_MSG(extack,
+                              "Invalid dsfield (tos): ECN bits must be 0");
                goto errout;
        }
+       rule4->dscp = inet_dsfield_to_dscp(frh->tos);
 
        /* split local/main if they are not already split */
        err = fib_unmerge(net);
@@ -270,7 +273,6 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
        rule4->srcmask = inet_make_mask(rule4->src_len);
        rule4->dst_len = frh->dst_len;
        rule4->dstmask = inet_make_mask(rule4->dst_len);
-       rule4->tos = frh->tos;
 
        net->ipv4.fib_has_custom_rules = true;
 
@@ -313,7 +315,7 @@ static int fib4_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh,
        if (frh->dst_len && (rule4->dst_len != frh->dst_len))
                return 0;
 
-       if (frh->tos && (rule4->tos != frh->tos))
+       if (frh->tos && inet_dscp_to_dsfield(rule4->dscp) != frh->tos)
                return 0;
 
 #ifdef CONFIG_IP_ROUTE_CLASSID
@@ -337,7 +339,7 @@ static int fib4_rule_fill(struct fib_rule *rule, struct sk_buff *skb,
 
        frh->dst_len = rule4->dst_len;
        frh->src_len = rule4->src_len;
-       frh->tos = rule4->tos;
+       frh->tos = inet_dscp_to_dsfield(rule4->dscp);
 
        if ((rule4->dst_len &&
             nla_put_in_addr(skb, FRA_DST, rule4->dst)) ||

diff --git a/tools/testing/selftests/net/fib_rule_tests.sh b/tools/testing/selftests/net/fib_rule_tests.sh
index d7a9ab3..4f70baa 100755 (executable)
--- a/tools/testing/selftests/net/fib_rule_tests.sh
+++ b/tools/testing/selftests/net/fib_rule_tests.sh
@@ -215,10 +215,25 @@ fib_rule4_test_match_n_redirect()
        log_test $? 0 "rule4 del by pref: $description"
 }
 
+fib_rule4_test_reject()
+{
+       local match="$1"
+       local rc
+
+       $IP rule add $match table $RTABLE 2>/dev/null
+       rc=$?
+       log_test $rc 2 "rule4 check: $match"
+
+       if [ $rc -eq 0 ]; then
+               $IP rule del $match table $RTABLE
+       fi
+}
+
 fib_rule4_test()
 {
        local getmatch
        local match
+       local cnt
 
        # setup the fib rule redirect route
        $IP route add table $RTABLE default via $GW_IP4 dev $DEV onlink
@@ -234,8 +249,21 @@ fib_rule4_test()
        fib_rule4_test_match_n_redirect "$match" "$match" "iif redirect to table"
        ip netns exec testns sysctl -qw net.ipv4.ip_forward=0
 
+       # Reject dsfield (tos) options which have ECN bits set
+       for cnt in $(seq 1 3); do
+               match="dsfield $cnt"
+               fib_rule4_test_reject "$match"
+       done
+
+       # Don't take ECN bits into account when matching on dsfield
        match="tos 0x10"
-       fib_rule4_test_match_n_redirect "$match" "$match" "tos redirect to table"
+       for cnt in "0x10" "0x11" "0x12" "0x13"; do
+               # Using option 'tos' instead of 'dsfield' as old iproute2
+               # versions don't support 'dsfield' in ip rule show.
+               getmatch="tos $cnt"
+               fib_rule4_test_match_n_redirect "$match" "$getmatch" \
+                                               "$getmatch redirect to table"
+       done
 
        match="fwmark 0x64"
        getmatch="mark 0x64"