Commit 563f8e97 authored by Guillaume Nault's avatar Guillaume Nault Committed by Jakub Kicinski

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: default avatarGuillaume Nault <gnault@redhat.com>
Acked-by: default avatarDavid Ahern <dsahern@kernel.org>
Reviewed-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent a410a0cf
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include <linux/list.h> #include <linux/list.h>
#include <linux/rcupdate.h> #include <linux/rcupdate.h>
#include <linux/export.h> #include <linux/export.h>
#include <net/inet_dscp.h>
#include <net/ip.h> #include <net/ip.h>
#include <net/route.h> #include <net/route.h>
#include <net/tcp.h> #include <net/tcp.h>
...@@ -35,7 +36,7 @@ struct fib4_rule { ...@@ -35,7 +36,7 @@ struct fib4_rule {
struct fib_rule common; struct fib_rule common;
u8 dst_len; u8 dst_len;
u8 src_len; u8 src_len;
u8 tos; dscp_t dscp;
__be32 src; __be32 src;
__be32 srcmask; __be32 srcmask;
__be32 dst; __be32 dst;
...@@ -49,7 +50,7 @@ static bool fib4_rule_matchall(const struct fib_rule *rule) ...@@ -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); 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 false;
return fib_rule_matchall(rule); return fib_rule_matchall(rule);
} }
...@@ -185,7 +186,7 @@ INDIRECT_CALLABLE_SCOPE int fib4_rule_match(struct fib_rule *rule, ...@@ -185,7 +186,7 @@ INDIRECT_CALLABLE_SCOPE int fib4_rule_match(struct fib_rule *rule,
((daddr ^ r->dst) & r->dstmask)) ((daddr ^ r->dst) & r->dstmask))
return 0; return 0;
if (r->tos && (r->tos != fl4->flowi4_tos)) if (r->dscp && r->dscp != inet_dsfield_to_dscp(fl4->flowi4_tos))
return 0; return 0;
if (rule->ip_proto && (rule->ip_proto != fl4->flowi4_proto)) 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, ...@@ -225,10 +226,12 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
int err = -EINVAL; int err = -EINVAL;
struct fib4_rule *rule4 = (struct fib4_rule *) rule; struct fib4_rule *rule4 = (struct fib4_rule *) rule;
if (frh->tos & ~IPTOS_TOS_MASK) { if (!inet_validate_dscp(frh->tos)) {
NL_SET_ERR_MSG(extack, "Invalid tos"); NL_SET_ERR_MSG(extack,
"Invalid dsfield (tos): ECN bits must be 0");
goto errout; goto errout;
} }
rule4->dscp = inet_dsfield_to_dscp(frh->tos);
/* split local/main if they are not already split */ /* split local/main if they are not already split */
err = fib_unmerge(net); err = fib_unmerge(net);
...@@ -270,7 +273,6 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb, ...@@ -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->srcmask = inet_make_mask(rule4->src_len);
rule4->dst_len = frh->dst_len; rule4->dst_len = frh->dst_len;
rule4->dstmask = inet_make_mask(rule4->dst_len); rule4->dstmask = inet_make_mask(rule4->dst_len);
rule4->tos = frh->tos;
net->ipv4.fib_has_custom_rules = true; 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, ...@@ -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)) if (frh->dst_len && (rule4->dst_len != frh->dst_len))
return 0; return 0;
if (frh->tos && (rule4->tos != frh->tos)) if (frh->tos && inet_dscp_to_dsfield(rule4->dscp) != frh->tos)
return 0; return 0;
#ifdef CONFIG_IP_ROUTE_CLASSID #ifdef CONFIG_IP_ROUTE_CLASSID
...@@ -337,7 +339,7 @@ static int fib4_rule_fill(struct fib_rule *rule, struct sk_buff *skb, ...@@ -337,7 +339,7 @@ static int fib4_rule_fill(struct fib_rule *rule, struct sk_buff *skb,
frh->dst_len = rule4->dst_len; frh->dst_len = rule4->dst_len;
frh->src_len = rule4->src_len; frh->src_len = rule4->src_len;
frh->tos = rule4->tos; frh->tos = inet_dscp_to_dsfield(rule4->dscp);
if ((rule4->dst_len && if ((rule4->dst_len &&
nla_put_in_addr(skb, FRA_DST, rule4->dst)) || nla_put_in_addr(skb, FRA_DST, rule4->dst)) ||
......
...@@ -215,10 +215,25 @@ fib_rule4_test_match_n_redirect() ...@@ -215,10 +215,25 @@ fib_rule4_test_match_n_redirect()
log_test $? 0 "rule4 del by pref: $description" 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() fib_rule4_test()
{ {
local getmatch local getmatch
local match local match
local cnt
# setup the fib rule redirect route # setup the fib rule redirect route
$IP route add table $RTABLE default via $GW_IP4 dev $DEV onlink $IP route add table $RTABLE default via $GW_IP4 dev $DEV onlink
...@@ -234,8 +249,21 @@ fib_rule4_test() ...@@ -234,8 +249,21 @@ fib_rule4_test()
fib_rule4_test_match_n_redirect "$match" "$match" "iif redirect to table" fib_rule4_test_match_n_redirect "$match" "$match" "iif redirect to table"
ip netns exec testns sysctl -qw net.ipv4.ip_forward=0 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" 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" match="fwmark 0x64"
getmatch="mark 0x64" getmatch="mark 0x64"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment