From d80d8d55734998a22a064e7cd59ffc66dcc86fb2 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 27 Apr 2018 14:06:35 -0400 Subject: [PATCH 1/3] opa_vnic: Just use skb_get_hash instead of skb_tx_hash This patch is meant to clean up how the opa_vnic is obtaining entropy from Tx packets. The code as it was written was claiming to get 16 bits of hash, but from what I can tell it was only ever actually getting 14 bits as it was limited to 0 - (2^15 - 1). It then was folding the result to get a 8 bit value for entropy. Instead of throwing away all that input I am cutting out the middle man and instead having the code call skb_get_hash directly and then folding the 32 bit value into a 8 bit value using a pair of shifts and XOR operations. Execution wise this new approach should provide more entropy and be faster since we are bypassing the reciprocal multiplication to reduce the 32b value to 16b and instead just using a shift/XOR combination. In addition we can drop the unneeded adapter value from the call to get the entropy since the netdev itself isn't even needed. Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- .../infiniband/ulp/opa_vnic/opa_vnic_encap.c | 19 +++++++++---------- .../ulp/opa_vnic/opa_vnic_internal.h | 2 +- .../infiniband/ulp/opa_vnic/opa_vnic_netdev.c | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.c index 4be3aef40bd2..267da8215e08 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.c +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.c @@ -443,17 +443,16 @@ static u8 opa_vnic_get_rc(struct __opa_veswport_info *info, } /* opa_vnic_calc_entropy - calculate the packet entropy */ -u8 opa_vnic_calc_entropy(struct opa_vnic_adapter *adapter, struct sk_buff *skb) +u8 opa_vnic_calc_entropy(struct sk_buff *skb) { - u16 hash16; + u32 hash = skb_get_hash(skb); - /* - * Get flow based 16-bit hash and then XOR the upper and lower bytes - * to get the entropy. - * __skb_tx_hash limits qcount to 16 bits. Hence, get 15-bit hash. - */ - hash16 = __skb_tx_hash(adapter->netdev, skb, BIT(15)); - return (u8)((hash16 >> 8) ^ (hash16 & 0xff)); + /* store XOR of all bytes in lower 8 bits */ + hash ^= hash >> 8; + hash ^= hash >> 16; + + /* return lower 8 bits as entropy */ + return (u8)(hash & 0xFF); } /* opa_vnic_get_def_port - get default port based on entropy */ @@ -490,7 +489,7 @@ void opa_vnic_encap_skb(struct opa_vnic_adapter *adapter, struct sk_buff *skb) hdr = skb_push(skb, OPA_VNIC_HDR_LEN); - entropy = opa_vnic_calc_entropy(adapter, skb); + entropy = opa_vnic_calc_entropy(skb); def_port = opa_vnic_get_def_port(adapter, entropy); len = opa_vnic_wire_length(skb); dlid = opa_vnic_get_dlid(adapter, skb, def_port); diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h index afd95f432262..43ac61ffef4a 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h @@ -299,7 +299,7 @@ struct opa_vnic_adapter *opa_vnic_add_netdev(struct ib_device *ibdev, void opa_vnic_rem_netdev(struct opa_vnic_adapter *adapter); void opa_vnic_encap_skb(struct opa_vnic_adapter *adapter, struct sk_buff *skb); u8 opa_vnic_get_vl(struct opa_vnic_adapter *adapter, struct sk_buff *skb); -u8 opa_vnic_calc_entropy(struct opa_vnic_adapter *adapter, struct sk_buff *skb); +u8 opa_vnic_calc_entropy(struct sk_buff *skb); void opa_vnic_process_vema_config(struct opa_vnic_adapter *adapter); void opa_vnic_release_mac_tbl(struct opa_vnic_adapter *adapter); void opa_vnic_query_mac_tbl(struct opa_vnic_adapter *adapter, diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c index ce57e0f10289..0c8aec62a425 100644 --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c @@ -104,7 +104,7 @@ static u16 opa_vnic_select_queue(struct net_device *netdev, struct sk_buff *skb, /* pass entropy and vl as metadata in skb */ mdata = skb_push(skb, sizeof(*mdata)); - mdata->entropy = opa_vnic_calc_entropy(adapter, skb); + mdata->entropy = opa_vnic_calc_entropy(skb); mdata->vl = opa_vnic_get_vl(adapter, skb); rc = adapter->rn_ops->ndo_select_queue(netdev, skb, accel_priv, fallback); From b86629ebd5447c1e263d89be765ba6fd747c5e4d Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 27 Apr 2018 14:06:45 -0400 Subject: [PATCH 2/3] mlx4: Don't bother using skb_tx_hash in mlx4_en_select_queue The code in the fallback path has supported XDP in conjunction with the Tx traffic classification for TCs for over a year now. So instead of just calling skb_tx_hash for every packet we are better off using the fallback since that will record the Tx queue to the socket and then that can be used instead of having to recompute the hash every time. Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c index 6b6853773848..0227786308af 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c @@ -694,7 +694,7 @@ u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb, u16 rings_p_up = priv->num_tx_rings_p_up; if (netdev_get_num_tc(dev)) - return skb_tx_hash(dev, skb); + return fallback(dev, skb); return fallback(dev, skb) % rings_p_up; } From 1b837d489e06a5289753ddbee99cfbc26d251d6d Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Fri, 27 Apr 2018 14:06:53 -0400 Subject: [PATCH 3/3] net: Revoke export for __skb_tx_hash, update it to just be static skb_tx_hash I am dropping the export of __skb_tx_hash as after my patches nobody is using it outside of the net/core/dev.c file. In addition I am renaming and repurposing it to just be a static declaration of skb_tx_hash since that was the only user for it at this point. By doing this the compiler can inline it into __netdev_pick_tx as that will improve performance. Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- include/linux/netdevice.h | 13 ------------- net/core/dev.c | 10 ++++------ 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 366c32891158..82f5a9aba578 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3213,19 +3213,6 @@ static inline int netif_set_xps_queue(struct net_device *dev, } #endif -u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb, - unsigned int num_tx_queues); - -/* - * Returns a Tx hash for the given packet when dev->real_num_tx_queues is used - * as a distribution range limit for the returned value. - */ -static inline u16 skb_tx_hash(const struct net_device *dev, - struct sk_buff *skb) -{ - return __skb_tx_hash(dev, skb, dev->real_num_tx_queues); -} - /** * netif_is_multiqueue - test if device has multiple transmit queues * @dev: network device diff --git a/net/core/dev.c b/net/core/dev.c index 0a2d46424069..25ceecfdd8fe 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2615,17 +2615,16 @@ EXPORT_SYMBOL(netif_device_attach); * Returns a Tx hash based on the given packet descriptor a Tx queues' number * to be used as a distribution range. */ -u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb, - unsigned int num_tx_queues) +static u16 skb_tx_hash(const struct net_device *dev, struct sk_buff *skb) { u32 hash; u16 qoffset = 0; - u16 qcount = num_tx_queues; + u16 qcount = dev->real_num_tx_queues; if (skb_rx_queue_recorded(skb)) { hash = skb_get_rx_queue(skb); - while (unlikely(hash >= num_tx_queues)) - hash -= num_tx_queues; + while (unlikely(hash >= qcount)) + hash -= qcount; return hash; } @@ -2638,7 +2637,6 @@ u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb, return (u16) reciprocal_scale(skb_get_hash(skb), qcount) + qoffset; } -EXPORT_SYMBOL(__skb_tx_hash); static void skb_warn_bad_offload(const struct sk_buff *skb) {