| Message ID | 20220110081537.82477-1-moshet@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RESEND,net] bonding: Fix extraction of ports from the packet headers | expand |
Resending my reply as I again forgot that Gmail's mobile app doesn't do plain text. Sorry about that. On Mon, Jan 10, 2022 at 9:16 AM Moshe Tal <moshet@nvidia.com> wrote: > > Wrong hash sends single stream to multiple output interfaces. > > The nhoff parameter is relative to skb->head, so convert it to be > relative to skb->data for using in skb_flow_get_ports(). ... > if (l34 && *ip_proto >= 0) > - fk->ports.ports = __skb_flow_get_ports(skb, *nhoff, *ip_proto, data, hlen); > + /* nhoff is relative to skb->head instead of the usual skb->data */ > + fk->ports.ports = skb_flow_get_ports(skb, *nhoff - skb_headroom(skb), *ip_proto); This will likely crash as skb can be NULL here when calculating the hash for a xdp_buff. You'll need to make sure this code also works for bond_xmit_hash_xdp, which passes a data pointer, but no skb to bond_flow_dissect. In what case was the original code broken? The flow dissector should've used the passed in "data" pointer, but I guess in some cases not enough data was in the linear region. The right fix is probably to make sure "nhoff" stays relative to skb->data. The optional skb pointer is rather unfortunate and bound to cause issues in the future. Perhaps might be worthwhile at some point to have a more abstract notion for a packet buffer, with xdp and skb implementations and a flow dissector for it? You can verify that this does not break the XDP bonding functionality by running the xdp_bonding bpf selftest ("vmtest.sh -t ./test_progs -t xdp_bonding" in tools/testing/selftests/bpf).
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 07fc603c2fa7..3189bd14c664 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3745,7 +3745,8 @@ static bool bond_flow_ip(struct sk_buff *skb, struct flow_keys *fk, const void * } if (l34 && *ip_proto >= 0) - fk->ports.ports = __skb_flow_get_ports(skb, *nhoff, *ip_proto, data, hlen); + /* nhoff is relative to skb->head instead of the usual skb->data */ + fk->ports.ports = skb_flow_get_ports(skb, *nhoff - skb_headroom(skb), *ip_proto); return true; }
Wrong hash sends single stream to multiple output interfaces. The nhoff parameter is relative to skb->head, so convert it to be relative to skb->data for using in skb_flow_get_ports(). Fixes: a815bde56b15 ("net, bonding: Refactor bond_xmit_hash for use with xdp_buff") Reviewed-by: Saeed Mahameed <saeedm@nvidia.com> Reviewed-by: Gal Pressman <gal@nvidia.com> Signed-off-by: Moshe Tal <moshet@nvidia.com> --- drivers/net/bonding/bond_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)