Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Matteo Croce <mcroce@redhat.com>
To:     netdev@vger.kernel.org
Cc:     Jay Vosburgh <j.vosburgh@gmail.com>,
        Veaceslav Falico <vfalico@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        "David S . Miller " <davem@davemloft.net>,
        Stanislav Fomichev <sdf@google.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Song Liu <songliubraving@fb.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Paul Blakey <paulb@mellanox.com>, linux-kernel@vger.kernel.org
Subject: [PATCH net-next 4/4] bonding: balance ICMP echoes in layer3+4 mode
Date:   Mon, 21 Oct 2019 22:09:48 +0200
Message-Id: <20191021200948.23775-5-mcroce@redhat.com>
In-Reply-To: <20191021200948.23775-1-mcroce@redhat.com>
References: <20191021200948.23775-1-mcroce@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The bonding uses the L4 ports to balance flows between slaves.
As the ICMP protocol has no ports, those packets are sent all to the
same device:

    # tcpdump -qltnni veth0 ip |sed 's/^/0: /' &
    # tcpdump -qltnni veth1 ip |sed 's/^/1: /' &
    # ping -qc1 192.168.0.2
    1: IP 192.168.0.1 > 192.168.0.2: ICMP echo request, id 315, seq 1, leng=
th 64
    1: IP 192.168.0.2 > 192.168.0.1: ICMP echo reply, id 315, seq 1, length=
 64
    # ping -qc1 192.168.0.2
    1: IP 192.168.0.1 > 192.168.0.2: ICMP echo request, id 316, seq 1, leng=
th 64
    1: IP 192.168.0.2 > 192.168.0.1: ICMP echo reply, id 316, seq 1, length=
 64
    # ping -qc1 192.168.0.2
    1: IP 192.168.0.1 > 192.168.0.2: ICMP echo request, id 317, seq 1, leng=
th 64
    1: IP 192.168.0.2 > 192.168.0.1: ICMP echo reply, id 317, seq 1, length=
 64

But some ICMP packets have an Identifier field which is
used to match packets within sessions, let's use this value in the hash
function to balance these packets between bond slaves:

    # ping -qc1 192.168.0.2
    0: IP 192.168.0.1 > 192.168.0.2: ICMP echo request, id 303, seq 1, leng=
th 64
    0: IP 192.168.0.2 > 192.168.0.1: ICMP echo reply, id 303, seq 1, length=
 64
    # ping -qc1 192.168.0.2
    1: IP 192.168.0.1 > 192.168.0.2: ICMP echo request, id 304, seq 1, leng=
th 64
    1: IP 192.168.0.2 > 192.168.0.1: ICMP echo reply, id 304, seq 1, length=
 64

Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 drivers/net/bonding/bond_main.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_mai=
n.c
index 21d8fcc83c9c..83afb03f4d07 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3267,6 +3267,8 @@ static bool bond_flow_dissect(struct bonding *bond, s=
truct sk_buff *skb,
 =09=09return skb_flow_dissect_flow_keys(skb, fk, 0);
=20
 =09fk->ports.ports =3D 0;
+=09fk->icmp.icmp =3D 0;
+=09fk->icmp.id =3D 0;
 =09noff =3D skb_network_offset(skb);
 =09if (skb->protocol =3D=3D htons(ETH_P_IP)) {
 =09=09if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph))))
@@ -3286,8 +3288,14 @@ static bool bond_flow_dissect(struct bonding *bond, =
struct sk_buff *skb,
 =09} else {
 =09=09return false;
 =09}
-=09if (bond->params.xmit_policy =3D=3D BOND_XMIT_POLICY_LAYER34 && proto >=
=3D 0)
-=09=09fk->ports.ports =3D skb_flow_get_ports(skb, noff, proto);
+=09if (bond->params.xmit_policy =3D=3D BOND_XMIT_POLICY_LAYER34 && proto >=
=3D 0) {
+=09=09if (proto =3D=3D IPPROTO_ICMP || proto =3D=3D IPPROTO_ICMPV6)
+=09=09=09skb_flow_get_icmp_tci(skb, &fk->icmp, skb->data,
+=09=09=09=09=09      skb_transport_offset(skb),
+=09=09=09=09=09      skb_headlen(skb));
+=09=09else
+=09=09=09fk->ports.ports =3D skb_flow_get_ports(skb, noff, proto);
+=09}
=20
 =09return true;
 }
@@ -3314,10 +3322,14 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_=
buff *skb)
 =09=09return bond_eth_hash(skb);
=20
 =09if (bond->params.xmit_policy =3D=3D BOND_XMIT_POLICY_LAYER23 ||
-=09    bond->params.xmit_policy =3D=3D BOND_XMIT_POLICY_ENCAP23)
+=09    bond->params.xmit_policy =3D=3D BOND_XMIT_POLICY_ENCAP23) {
 =09=09hash =3D bond_eth_hash(skb);
-=09else
-=09=09hash =3D (__force u32)flow.ports.ports;
+=09} else {
+=09=09if (flow.icmp.id)
+=09=09=09memcpy(&hash, &flow.icmp, sizeof(hash));
+=09=09else
+=09=09=09memcpy(&hash, &flow.ports.ports, sizeof(hash));
+=09}
 =09hash ^=3D (__force u32)flow_get_u32_dst(&flow) ^
 =09=09(__force u32)flow_get_u32_src(&flow);
 =09hash ^=3D (hash >> 16);
--=20
2.21.0