On Thu, Jun 10, 2021 at 1:29 AM Jay Vosburgh [off-list ref] wrote:

        The design adds logic around a bpf_bond_redirect_enabled_key
static key in the BPF core functions dev_map_enqueue_multi,
dev_map_redirect_multi and bpf_prog_run_xdp.  Is this something that is
correctly implemented as a special case just for bonding (i.e., it will
never ever have to be extended), or is it possible that other
upper/lower type software devices will have similar XDP functionality
added in the future, e.g., bridge, VLAN, etc?

Good point. For example the "team" driver would basically need pretty
much the same implementation. For that just using non-bond naming
would be enough. I don't think there's much of a cost for doing a more
generic mechanism, e.g. xdp "upper intercept" hook in netdev_ops, so
I'll try that out. At the very least I'll change the naming.

...

quoted

@@ -3543,26 +3614,30 @@ static u32 bond_vlan_srcmac_hash(struct sk_buff *skb)

}

/* Extract the appropriate headers based on bond's xmit policy */
-static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
+static bool bond_flow_dissect(struct bonding *bond,
+                            struct sk_buff *skb,
+                            const void *data,
+                            __be16 l2_proto,
+                            int nhoff,
+                            int hlen,
                            struct flow_keys *fk)

        Please compact the argument list down to fewer lines, in
conformance with usual coding practice in the kernel.  The above style
of formatting occurs multiple times in this patch, both in function
declarations and function calls.

Thanks will do.

...

quoted

-/**
- * bond_xmit_hash - generate a hash value based on the xmit policy
- * @bond: bonding device
- * @skb: buffer to use for headers
- *
- * This function will extract the necessary headers from the skb buffer and use
- * them to generate a hash based on the xmit_policy set in the bonding device
+/* Generate hash based on xmit policy. If @skb is given it is used to linearize
+ * the data as required, but this function can be used without it.

        Please don't remove kernel-doc formatting; add your new
parameters to the documentation.

The comment and the function declaration were untouched (see further
below in patch).  I only introduced the common helper __bond_xmit_hash
used from bond_xmit_hash and bond_xmit_hash_xdp. Unfortunately the
generated diff was a bit confusing. I'll try and generate cleaner
diffs in the future.

quoted

 */
-u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
+static u32 __bond_xmit_hash(struct bonding *bond,
+                          struct sk_buff *skb,
+                          const void *data,
+                          __be16 l2_proto,
+                          int mhoff,
+                          int nhoff,
+                          int hlen)
{
      struct flow_keys flow;
      u32 hash;

-      if (bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP34 &&
-          skb->l4_hash)
-              return skb->hash;
-
      if (bond->params.xmit_policy == BOND_XMIT_POLICY_VLAN_SRCMAC)
-              return bond_vlan_srcmac_hash(skb);
+              return bond_vlan_srcmac_hash(skb, data, mhoff, hlen);

      if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
-          !bond_flow_dissect(bond, skb, &flow))
-              return bond_eth_hash(skb);
+          !bond_flow_dissect(bond, skb, data, l2_proto, nhoff, hlen, &flow))
+              return bond_eth_hash(skb, data, mhoff, hlen);

      if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
          bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23) {
-              hash = bond_eth_hash(skb);
+              hash = bond_eth_hash(skb, data, mhoff, hlen);
      } else {
              if (flow.icmp.id)
                      memcpy(&hash, &flow.icmp, sizeof(hash));

@@ -3638,6 +3708,48 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
      return bond_ip_hash(hash, &flow);

}

+/**
+ * bond_xmit_hash_skb - generate a hash value based on the xmit policy
+ * @bond: bonding device
+ * @skb: buffer to use for headers
+ *
+ * This function will extract the necessary headers from the skb buffer and use
+ * them to generate a hash based on the xmit_policy set in the bonding device
+ */
+u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
+{
+      if (bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP34 &&
+          skb->l4_hash)
+              return skb->hash;
+
+      return __bond_xmit_hash(bond, skb, skb->head, skb->protocol,
+                              skb->mac_header,
+                              skb->network_header,
+                              skb_headlen(skb));
+}

...