[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <14149.1625260463@famine>
Date: Fri, 02 Jul 2021 14:14:23 -0700
From: Jay Vosburgh <jay.vosburgh@...onical.com>
To: Taehee Yoo <ap420073@...il.com>
cc: davem@...emloft.net, kuba@...nel.org, vfalico@...il.com,
andy@...yhouse.net, jesse.brandeburg@...el.com,
anthony.l.nguyen@...el.com, jarod@...hat.com,
netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net 6/8] bonding: disallow setting nested bonding + ipsec offload
Taehee Yoo <ap420073@...il.com> wrote:
>bonding interface can be nested and it supports ipsec offload.
>So, it allows setting the nested bonding + ipsec scenario.
>But code does not support this scenario.
>So, it should be disallowed.
>
>interface graph:
>bond2
> |
>bond1
> |
>eth0
>
>The nested bonding + ipsec offload may not a real usecase.
>So, disallowing this is fine.
Is a stack like "bond1 -> VLAN.XX -> bond2 -> eth0" also a
problem? I don't believe the change below will detect this
configuration.
-J
>Fixes: 18cb261afd7b ("bonding: support hardware encryption offload to slaves")
>Signed-off-by: Taehee Yoo <ap420073@...il.com>
>---
> drivers/net/bonding/bond_main.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 7659e1fab19e..f268e67cb2f0 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -419,8 +419,9 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs)
> xs->xso.real_dev = slave->dev;
> bond->xs = xs;
>
>- if (!(slave->dev->xfrmdev_ops
>- && slave->dev->xfrmdev_ops->xdo_dev_state_add)) {
>+ if (!slave->dev->xfrmdev_ops ||
>+ !slave->dev->xfrmdev_ops->xdo_dev_state_add ||
>+ netif_is_bond_master(slave->dev)) {
> slave_warn(bond_dev, slave->dev, "Slave does not support ipsec offload\n");
> rcu_read_unlock();
> return -EINVAL;
>@@ -453,8 +454,9 @@ static void bond_ipsec_del_sa(struct xfrm_state *xs)
>
> xs->xso.real_dev = slave->dev;
>
>- if (!(slave->dev->xfrmdev_ops
>- && slave->dev->xfrmdev_ops->xdo_dev_state_delete)) {
>+ if (!slave->dev->xfrmdev_ops ||
>+ !slave->dev->xfrmdev_ops->xdo_dev_state_delete ||
>+ netif_is_bond_master(slave->dev)) {
> slave_warn(bond_dev, slave->dev, "%s: no slave xdo_dev_state_delete\n", __func__);
> goto out;
> }
>@@ -479,8 +481,9 @@ static bool bond_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
> if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)
> return true;
>
>- if (!(slave_dev->xfrmdev_ops
>- && slave_dev->xfrmdev_ops->xdo_dev_offload_ok)) {
>+ if (!slave_dev->xfrmdev_ops ||
>+ !slave_dev->xfrmdev_ops->xdo_dev_offload_ok ||
>+ netif_is_bond_master(slave_dev)) {
> slave_warn(bond_dev, slave_dev, "%s: no slave xdo_dev_offload_ok\n", __func__);
> return false;
> }
>--
>2.17.1
>
---
-Jay Vosburgh, jay.vosburgh@...onical.com
Powered by blists - more mailing lists