Re: [PATCH v4 net-next] net: multipath routing: configurable seed

From: Pavel Balaev
Date: Tue Apr 27 2021 - 05:54:19 EST


On Mon, Apr 26, 2021 at 09:21:53PM -0600, David Ahern wrote:
> On 4/23/21 6:44 AM, Balaev Pavel wrote:
> > Ability for a user to assign seed value to multipath route hashes.
> > Now kernel uses random seed value to prevent hash-flooding DoS attacks;
> > however, it disables some use cases, f.e:
> >
> > +-------+ +------+ +--------+
> > | |-eth0---| FW0 |---eth0-| |
> > | | +------+ | |
> > | GW0 |ECMP ECMP| GW1 |
> > | | +------+ | |
> > | |-eth1---| FW1 |---eth1-| |
> > +-------+ +------+ +--------+
> >
> > In this use case, two ECMP routers balance traffic between two firewalls.
> > If some flow transmits a response over a different channel than request,
> > such flow will be dropped, because keep-state rules are created on
> > the other firewall.
> >
> > This patch adds sysctl variable: net.ipv4|ipv6.fib_multipath_hash_seed.
> > User can set the same seed value on GW0 and GW1 for traffic to be
> > mirror-balanced. By default, random value is used.
> >
> > Signed-off-by: Balaev Pavel <balaevpa@xxxxxxxxxxx>
> > ---
> > Documentation/networking/ip-sysctl.rst | 14 +
> > include/net/flow_dissector.h | 4 +
> > include/net/netns/ipv4.h | 2 +
> > include/net/netns/ipv6.h | 3 +
> > net/core/flow_dissector.c | 9 +
> > net/ipv4/route.c | 10 +-
> > net/ipv4/sysctl_net_ipv4.c | 97 +++++
> > net/ipv6/route.c | 10 +-
> > net/ipv6/sysctl_net_ipv6.c | 96 +++++
> > .../testing/selftests/net/forwarding/Makefile | 1 +
> > tools/testing/selftests/net/forwarding/lib.sh | 41 +++
> > .../net/forwarding/router_mpath_seed.sh | 347 ++++++++++++++++++
> > 12 files changed, 632 insertions(+), 2 deletions(-)
> > create mode 100755 tools/testing/selftests/net/forwarding/router_mpath_seed.sh
>
> this really needs to be multiple patches. At a minimum 1 for ipv4, 1 for
> ipv6 and 1 for the test script (thank you for adding that).
>
> [ cc'ed Ido since most of the tests under
> tools/testing/selftests/net/forwarding come from him and team ]

OK, I will create 3 patches. Thanks for the advice.

> >
> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index 9701906f6..d1a67e6fe 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -100,6 +100,20 @@ fib_multipath_hash_policy - INTEGER
> > - 1 - Layer 4
> > - 2 - Layer 3 or inner Layer 3 if present
> >
> > +fib_multipath_hash_seed - STRING
> > + Controls seed value for multipath route hashes. By default
> > + random value is used. Only valid for kernels built with
> > + CONFIG_IP_ROUTE_MULTIPATH enabled.
> > +
> > + Valid format: two hex values set off with comma or "random"
> > + keyword.
> > +
> > + Example to generate the seed value::
> > +
> > + RAND=$(openssl rand -hex 16) && echo "${RAND:0:16},${RAND:16:16}"
> > +
> > + Default: "random"
> > +
> > fib_sync_mem - UNSIGNED INTEGER
> > Amount of dirty memory from fib entries that can be backlogged before
> > synchronize_rcu is forced.
> > diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> > index ffd386ea0..2bd4e28de 100644
> > --- a/include/net/flow_dissector.h
> > +++ b/include/net/flow_dissector.h
> > @@ -348,6 +348,10 @@ static inline bool flow_keys_have_l4(const struct flow_keys *keys)
> > }
> >
> > u32 flow_hash_from_keys(struct flow_keys *keys);
> > +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> > +u32 flow_multipath_hash_from_keys(struct flow_keys *keys,
> > + const siphash_key_t *seed);
>
> column alignment looks off here ^^^^ and a few other places; please
> correct in the next version.
>
After running "scripts/checkpatch.pl" I got warnings about alignment.
So I run checkpatch.pl --fix and fixed alignment as a script did.
So warnings goes away. I don't get the rules of alignment, can you
tell me the right way?