* [Patch net-next 04/12] net: hns3: remove some ops in struct hns3_nic_ops
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Yunsheng Lin, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
The fill_desc ops has only one implementation, and
get_rxd_bnum has not been used, so this patch removes
them.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 11 +++++------
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 3 ---
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a84060fd..33ac732 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1279,8 +1279,8 @@ netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
next_to_use_head = ring->next_to_use;
- ret = priv->ops.fill_desc(ring, skb, size, seg_num == 1 ? 1 : 0,
- DESC_TYPE_SKB);
+ ret = hns3_fill_desc(ring, skb, size, seg_num == 1 ? 1 : 0,
+ DESC_TYPE_SKB);
if (ret)
goto head_fill_err;
@@ -1290,9 +1290,9 @@ netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
frag = &skb_shinfo(skb)->frags[i - 1];
size = skb_frag_size(frag);
- ret = priv->ops.fill_desc(ring, frag, size,
- seg_num - 1 == i ? 1 : 0,
- DESC_TYPE_PAGE);
+ ret = hns3_fill_desc(ring, frag, size,
+ seg_num - 1 == i ? 1 : 0,
+ DESC_TYPE_PAGE);
if (ret)
goto frag_fill_err;
@@ -3579,7 +3579,6 @@ static void hns3_nic_set_priv_ops(struct net_device *netdev)
{
struct hns3_nic_priv *priv = netdev_priv(netdev);
- priv->ops.fill_desc = hns3_fill_desc;
if ((netdev->features & NETIF_F_TSO) ||
(netdev->features & NETIF_F_TSO6))
priv->ops.maybe_stop_tx = hns3_nic_maybe_stop_tso;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 5c41465..3a1e487 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -444,11 +444,8 @@ struct hns3_nic_ring_data {
};
struct hns3_nic_ops {
- int (*fill_desc)(struct hns3_enet_ring *ring, void *priv,
- int size, int frag_end, enum hns_desc_type type);
int (*maybe_stop_tx)(struct sk_buff **out_skb,
int *bnum, struct hns3_enet_ring *ring);
- void (*get_rxd_bnum)(u32 bnum_flag, int *out_bnum);
};
enum hns3_flow_level_range {
--
2.7.4
^ permalink raw reply related
* [Patch net-next 01/12] net: hns3: add xps setting support for hns3 driver
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Yunsheng Lin, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
This patch adds xps setting support for hns3 driver based on
the interrupt affinity info.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 40b1722..d75977c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -378,6 +378,29 @@ static int hns3_nic_net_up(struct net_device *netdev)
return ret;
}
+static void hns3_config_xps(struct hns3_nic_priv *priv)
+{
+ int i;
+
+ for (i = 0; i < priv->vector_num; i++) {
+ struct hns3_enet_tqp_vector *tqp_vector = &priv->tqp_vector[i];
+ struct hns3_enet_ring *ring = tqp_vector->tx_group.ring;
+
+ while (ring) {
+ int ret;
+
+ ret = netif_set_xps_queue(priv->netdev,
+ &tqp_vector->affinity_mask,
+ ring->tqp->tqp_index);
+ if (ret)
+ netdev_warn(priv->netdev,
+ "set xps queue failed: %d", ret);
+
+ ring = ring->next;
+ }
+ }
+}
+
static int hns3_nic_net_open(struct net_device *netdev)
{
struct hns3_nic_priv *priv = netdev_priv(netdev);
@@ -410,6 +433,7 @@ static int hns3_nic_net_open(struct net_device *netdev)
if (h->ae_algo->ops->set_timer_task)
h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
+ hns3_config_xps(priv);
return 0;
}
--
2.7.4
^ permalink raw reply related
* [Patch net-next 06/12] net: hns3: replace hnae3_set_bit and hnae3_set_field in data path
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Yunsheng Lin, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
hnae3_set_bit and hnae3_set_field masks the data before setting
the field or bit, which is unnecessary because the data is already
zero initialized.
Suggested-by: John Garry <john.garry@huawei.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 120 ++++++++++--------------
1 file changed, 51 insertions(+), 69 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 50bfdba..ce9878b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -21,6 +21,8 @@
#include "hnae3.h"
#include "hns3_enet.h"
+#define hns3_set_field(origin, shift, val) ((origin) |= ((val) << (shift)))
+
static void hns3_clear_all_ring(struct hnae3_handle *h);
static void hns3_force_clear_all_rx_ring(struct hnae3_handle *h);
static void hns3_remove_hw_addr(struct net_device *netdev);
@@ -667,8 +669,7 @@ static int hns3_set_tso(struct sk_buff *skb, u32 *paylen,
/* find the txbd field values */
*paylen = skb->len - hdr_len;
- hnae3_set_bit(*type_cs_vlan_tso,
- HNS3_TXD_TSO_B, 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_TSO_B, 1);
/* get MSS for TSO */
*mss = skb_shinfo(skb)->gso_size;
@@ -747,21 +748,19 @@ static void hns3_set_l2l3l4_len(struct sk_buff *skb, u8 ol4_proto,
/* compute L2 header size for normal packet, defined in 2 Bytes */
l2_len = l3.hdr - skb->data;
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L2LEN_M,
- HNS3_TXD_L2LEN_S, l2_len >> 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L2LEN_S, l2_len >> 1);
/* tunnel packet*/
if (skb->encapsulation) {
/* compute OL2 header size, defined in 2 Bytes */
ol2_len = l2_len;
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_L2LEN_M,
- HNS3_TXD_L2LEN_S, ol2_len >> 1);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_L2LEN_S, ol2_len >> 1);
/* compute OL3 header size, defined in 4 Bytes */
ol3_len = l4.hdr - l3.hdr;
- hnae3_set_field(*ol_type_vlan_len_msec, HNS3_TXD_L3LEN_M,
- HNS3_TXD_L3LEN_S, ol3_len >> 2);
+ hns3_set_field(*ol_type_vlan_len_msec, HNS3_TXD_L3LEN_S,
+ ol3_len >> 2);
/* MAC in UDP, MAC in GRE (0x6558)*/
if ((ol4_proto == IPPROTO_UDP) || (ol4_proto == IPPROTO_GRE)) {
@@ -770,17 +769,16 @@ static void hns3_set_l2l3l4_len(struct sk_buff *skb, u8 ol4_proto,
/* compute OL4 header size, defined in 4 Bytes. */
ol4_len = l2_hdr - l4.hdr;
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_L4LEN_M, HNS3_TXD_L4LEN_S,
- ol4_len >> 2);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_L4LEN_S, ol4_len >> 2);
/* switch IP header ptr from outer to inner header */
l3.hdr = skb_inner_network_header(skb);
/* compute inner l2 header size, defined in 2 Bytes. */
l2_len = l3.hdr - l2_hdr;
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L2LEN_M,
- HNS3_TXD_L2LEN_S, l2_len >> 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L2LEN_S,
+ l2_len >> 1);
} else {
/* skb packet types not supported by hardware,
* txbd len fild doesn't be filled.
@@ -796,24 +794,21 @@ static void hns3_set_l2l3l4_len(struct sk_buff *skb, u8 ol4_proto,
/* compute inner(/normal) L3 header size, defined in 4 Bytes */
l3_len = l4.hdr - l3.hdr;
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3LEN_M,
- HNS3_TXD_L3LEN_S, l3_len >> 2);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3LEN_S, l3_len >> 2);
/* compute inner(/normal) L4 header size, defined in 4 Bytes */
switch (l4_proto) {
case IPPROTO_TCP:
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_M,
- HNS3_TXD_L4LEN_S, l4.tcp->doff);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_S,
+ l4.tcp->doff);
break;
case IPPROTO_SCTP:
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_M,
- HNS3_TXD_L4LEN_S,
- (sizeof(struct sctphdr) >> 2));
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_S,
+ (sizeof(struct sctphdr) >> 2));
break;
case IPPROTO_UDP:
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_M,
- HNS3_TXD_L4LEN_S,
- (sizeof(struct udphdr) >> 2));
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4LEN_S,
+ (sizeof(struct udphdr) >> 2));
break;
default:
/* skb packet types not supported by hardware,
@@ -858,34 +853,30 @@ static int hns3_set_l3l4_type_csum(struct sk_buff *skb, u8 ol4_proto,
/* define outer network header type.*/
if (skb->protocol == htons(ETH_P_IP)) {
if (skb_is_gso(skb))
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_OL3T_M,
- HNS3_TXD_OL3T_S,
- HNS3_OL3T_IPV4_CSUM);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_OL3T_S,
+ HNS3_OL3T_IPV4_CSUM);
else
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_OL3T_M,
- HNS3_TXD_OL3T_S,
- HNS3_OL3T_IPV4_NO_CSUM);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_OL3T_S,
+ HNS3_OL3T_IPV4_NO_CSUM);
} else if (skb->protocol == htons(ETH_P_IPV6)) {
- hnae3_set_field(*ol_type_vlan_len_msec, HNS3_TXD_OL3T_M,
- HNS3_TXD_OL3T_S, HNS3_OL3T_IPV6);
+ hns3_set_field(*ol_type_vlan_len_msec, HNS3_TXD_OL3T_S,
+ HNS3_OL3T_IPV6);
}
/* define tunnel type(OL4).*/
switch (l4_proto) {
case IPPROTO_UDP:
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_TUNTYPE_M,
- HNS3_TXD_TUNTYPE_S,
- HNS3_TUN_MAC_IN_UDP);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_TUNTYPE_S,
+ HNS3_TUN_MAC_IN_UDP);
break;
case IPPROTO_GRE:
- hnae3_set_field(*ol_type_vlan_len_msec,
- HNS3_TXD_TUNTYPE_M,
- HNS3_TXD_TUNTYPE_S,
- HNS3_TUN_NVGRE);
+ hns3_set_field(*ol_type_vlan_len_msec,
+ HNS3_TXD_TUNTYPE_S,
+ HNS3_TUN_NVGRE);
break;
default:
/* drop the skb tunnel packet if hardware don't support,
@@ -906,43 +897,37 @@ static int hns3_set_l3l4_type_csum(struct sk_buff *skb, u8 ol4_proto,
}
if (l3.v4->version == 4) {
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3T_M,
- HNS3_TXD_L3T_S, HNS3_L3T_IPV4);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3T_S,
+ HNS3_L3T_IPV4);
/* the stack computes the IP header already, the only time we
* need the hardware to recompute it is in the case of TSO.
*/
if (skb_is_gso(skb))
- hnae3_set_bit(*type_cs_vlan_tso, HNS3_TXD_L3CS_B, 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3CS_B, 1);
} else if (l3.v6->version == 6) {
- hnae3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3T_M,
- HNS3_TXD_L3T_S, HNS3_L3T_IPV6);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L3T_S,
+ HNS3_L3T_IPV6);
}
switch (l4_proto) {
case IPPROTO_TCP:
- hnae3_set_bit(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
- hnae3_set_field(*type_cs_vlan_tso,
- HNS3_TXD_L4T_M,
- HNS3_TXD_L4T_S,
- HNS3_L4T_TCP);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4T_S,
+ HNS3_L4T_TCP);
break;
case IPPROTO_UDP:
if (hns3_tunnel_csum_bug(skb))
break;
- hnae3_set_bit(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
- hnae3_set_field(*type_cs_vlan_tso,
- HNS3_TXD_L4T_M,
- HNS3_TXD_L4T_S,
- HNS3_L4T_UDP);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4T_S,
+ HNS3_L4T_UDP);
break;
case IPPROTO_SCTP:
- hnae3_set_bit(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
- hnae3_set_field(*type_cs_vlan_tso,
- HNS3_TXD_L4T_M,
- HNS3_TXD_L4T_S,
- HNS3_L4T_SCTP);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4CS_B, 1);
+ hns3_set_field(*type_cs_vlan_tso, HNS3_TXD_L4T_S,
+ HNS3_L4T_SCTP);
break;
default:
/* drop the skb tunnel packet if hardware don't support,
@@ -964,11 +949,8 @@ static int hns3_set_l3l4_type_csum(struct sk_buff *skb, u8 ol4_proto,
static void hns3_set_txbd_baseinfo(u16 *bdtp_fe_sc_vld_ra_ri, int frag_end)
{
/* Config bd buffer end */
- hnae3_set_field(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_BDTYPE_M,
- HNS3_TXD_BDTYPE_S, 0);
- hnae3_set_bit(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_FE_B, !!frag_end);
- hnae3_set_bit(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_VLD_B, 1);
- hnae3_set_field(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_SC_M, HNS3_TXD_SC_S, 0);
+ hns3_set_field(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_FE_B, !!frag_end);
+ hns3_set_field(*bdtp_fe_sc_vld_ra_ri, HNS3_TXD_VLD_B, 1);
}
static int hns3_fill_desc_vtags(struct sk_buff *skb,
@@ -1001,10 +983,10 @@ static int hns3_fill_desc_vtags(struct sk_buff *skb,
* and use inner_vtag in one tag case.
*/
if (skb->protocol == htons(ETH_P_8021Q)) {
- hnae3_set_bit(*out_vlan_flag, HNS3_TXD_OVLAN_B, 1);
+ hns3_set_field(*out_vlan_flag, HNS3_TXD_OVLAN_B, 1);
*out_vtag = vlan_tag;
} else {
- hnae3_set_bit(*inner_vlan_flag, HNS3_TXD_VLAN_B, 1);
+ hns3_set_field(*inner_vlan_flag, HNS3_TXD_VLAN_B, 1);
*inner_vtag = vlan_tag;
}
} else if (skb->protocol == htons(ETH_P_8021Q)) {
--
2.7.4
^ permalink raw reply related
* [Patch net-next 02/12] net: hns3: avoid mult + div op in critical data path
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Yunsheng Lin, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
This patch uses shift offset to avoid doing mult and div operation.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 15 ++++++++-------
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 3 +++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index d75977c..14c3aa7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -658,7 +658,7 @@ static int hns3_set_tso(struct sk_buff *skb, u32 *paylen,
/* normal or tunnel packet*/
l4_offset = l4.hdr - skb->data;
- hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ hdr_len = (l4.tcp->doff << 2) + l4_offset;
/* remove payload length from inner pseudo checksum when tso*/
l4_paylen = skb->len - l4_offset;
@@ -1100,8 +1100,8 @@ static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv,
desc_cb->length = size;
- frag_buf_num = (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE;
- sizeoflast = size % HNS3_MAX_BD_SIZE;
+ frag_buf_num = (size + HNS3_MAX_BD_SIZE - 1) >> HNS3_MAX_BD_SIZE_OFFSET;
+ sizeoflast = size & HNS3_TX_LAST_SIZE_M;
sizeoflast = sizeoflast ? sizeoflast : HNS3_MAX_BD_SIZE;
/* When frag size is bigger than hardware limit, split this frag */
@@ -1145,14 +1145,14 @@ static int hns3_nic_maybe_stop_tso(struct sk_buff **out_skb, int *bnum,
int i;
size = skb_headlen(skb);
- buf_num = (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE;
+ buf_num = (size + HNS3_MAX_BD_SIZE - 1) >> HNS3_MAX_BD_SIZE_OFFSET;
frag_num = skb_shinfo(skb)->nr_frags;
for (i = 0; i < frag_num; i++) {
frag = &skb_shinfo(skb)->frags[i];
size = skb_frag_size(frag);
- bdnum_for_frag =
- (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE;
+ bdnum_for_frag = (size + HNS3_MAX_BD_SIZE - 1) >>
+ HNS3_MAX_BD_SIZE_OFFSET;
if (bdnum_for_frag > HNS3_MAX_BD_PER_FRAG)
return -ENOMEM;
@@ -1160,7 +1160,8 @@ static int hns3_nic_maybe_stop_tso(struct sk_buff **out_skb, int *bnum,
}
if (unlikely(buf_num > HNS3_MAX_BD_PER_FRAG)) {
- buf_num = (skb->len + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE;
+ buf_num = (skb->len + HNS3_MAX_BD_SIZE - 1) >>
+ HNS3_MAX_BD_SIZE_OFFSET;
if (ring_space(ring) < buf_num)
return -EBUSY;
/* manual split the send packet */
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 71ff8f4..5c41465 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -184,6 +184,8 @@ enum hns3_nic_state {
#define HNS3_TXD_MSS_S 0
#define HNS3_TXD_MSS_M (0x3fff << HNS3_TXD_MSS_S)
+#define HNS3_TX_LAST_SIZE_M 0xffff
+
#define HNS3_VECTOR_TX_IRQ BIT_ULL(0)
#define HNS3_VECTOR_RX_IRQ BIT_ULL(1)
@@ -191,6 +193,7 @@ enum hns3_nic_state {
#define HNS3_VECTOR_INITED 1
#define HNS3_MAX_BD_SIZE 65535
+#define HNS3_MAX_BD_SIZE_OFFSET 16
#define HNS3_MAX_BD_PER_FRAG 8
#define HNS3_MAX_BD_PER_PKT MAX_SKB_FRAGS
--
2.7.4
^ permalink raw reply related
* [Patch net-next 03/12] net: hns3: limit some variable scope in critical data path
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Yunsheng Lin, Huazhong tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
This patch limits some variables' scope as much as possible in
hns3_fill_desc.
Also, only set l3_type and l4_type when necessary.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 35 +++++++++++--------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 14c3aa7..a84060fd 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1029,26 +1029,21 @@ static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv,
struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_use];
struct hns3_desc *desc = &ring->desc[ring->next_to_use];
struct device *dev = ring_to_dev(ring);
- u32 ol_type_vlan_len_msec = 0;
u16 bdtp_fe_sc_vld_ra_ri = 0;
struct skb_frag_struct *frag;
unsigned int frag_buf_num;
- u32 type_cs_vlan_tso = 0;
- struct sk_buff *skb;
- u16 inner_vtag = 0;
- u16 out_vtag = 0;
- unsigned int k;
- int sizeoflast;
- u32 paylen = 0;
+ int k, sizeoflast;
dma_addr_t dma;
- u16 mss = 0;
- u8 ol4_proto;
- u8 il4_proto;
- int ret;
if (type == DESC_TYPE_SKB) {
- skb = (struct sk_buff *)priv;
- paylen = skb->len;
+ struct sk_buff *skb = (struct sk_buff *)priv;
+ u32 ol_type_vlan_len_msec = 0;
+ u32 type_cs_vlan_tso = 0;
+ u32 paylen = skb->len;
+ u16 inner_vtag = 0;
+ u16 out_vtag = 0;
+ u16 mss = 0;
+ int ret;
ret = hns3_fill_desc_vtags(skb, ring, &type_cs_vlan_tso,
&ol_type_vlan_len_msec,
@@ -1057,6 +1052,8 @@ static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv,
return ret;
if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ u8 ol4_proto, il4_proto;
+
skb_reset_mac_len(skb);
ret = hns3_get_l4_protocol(skb, &ol4_proto, &il4_proto);
@@ -2353,11 +2350,6 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
return;
}
- l3_type = hnae3_get_field(l234info, HNS3_RXD_L3ID_M,
- HNS3_RXD_L3ID_S);
- l4_type = hnae3_get_field(l234info, HNS3_RXD_L4ID_M,
- HNS3_RXD_L4ID_S);
-
ol4_type = hnae3_get_field(l234info, HNS3_RXD_OL4ID_M,
HNS3_RXD_OL4ID_S);
switch (ol4_type) {
@@ -2366,6 +2358,11 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
skb->csum_level = 1;
/* fall through */
case HNS3_OL4_TYPE_NO_TUN:
+ l3_type = hnae3_get_field(l234info, HNS3_RXD_L3ID_M,
+ HNS3_RXD_L3ID_S);
+ l4_type = hnae3_get_field(l234info, HNS3_RXD_L4ID_M,
+ HNS3_RXD_L4ID_S);
+
/* Can checksum ipv4 or ipv6 + UDP/TCP/SCTP packets */
if ((l3_type == HNS3_L3_TYPE_IPV4 ||
l3_type == HNS3_L3_TYPE_IPV6) &&
--
2.7.4
^ permalink raw reply related
* [Patch net-next 10/12] net: hns3: fix get VF RSS issue
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Jian Shen, Peng Li, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Jian Shen <shenjian15@huawei.com>
For revision 0x20, VF shares the same RSS config with PF.
In original codes, it always return 0 when query RSS hash
key for VF. This patch fixes it by return the hash key
got from PF.
Fixes: 374ad291762a ("net: hns3: net: hns3: Add RSS general configuration support for VF")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 25 +++++++++++
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 50 +++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
index ca056b3..306a23e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
@@ -490,6 +490,24 @@ static int hclge_get_queue_id_in_pf(struct hclge_vport *vport,
return hclge_gen_resp_to_vf(vport, mbx_req, 0, resp_data, 2);
}
+static int hclge_get_rss_key(struct hclge_vport *vport,
+ struct hclge_mbx_vf_to_pf_cmd *mbx_req)
+{
+#define HCLGE_RSS_MBX_RESP_LEN 8
+ u8 resp_data[HCLGE_RSS_MBX_RESP_LEN];
+ struct hclge_dev *hdev = vport->back;
+ u8 index;
+
+ index = mbx_req->msg[2];
+
+ memcpy(&resp_data[0],
+ &hdev->vport[0].rss_hash_key[index * HCLGE_RSS_MBX_RESP_LEN],
+ HCLGE_RSS_MBX_RESP_LEN);
+
+ return hclge_gen_resp_to_vf(vport, mbx_req, 0, resp_data,
+ HCLGE_RSS_MBX_RESP_LEN);
+}
+
static bool hclge_cmd_crq_empty(struct hclge_hw *hw)
{
u32 tail = hclge_read_dev(hw, HCLGE_NIC_CRQ_TAIL_REG);
@@ -625,6 +643,13 @@ void hclge_mbx_handler(struct hclge_dev *hdev)
"PF failed(%d) to get qid for VF\n",
ret);
break;
+ case HCLGE_MBX_GET_RSS_KEY:
+ ret = hclge_get_rss_key(vport, req);
+ if (ret)
+ dev_err(&hdev->pdev->dev,
+ "PF fail(%d) to get rss key for VF\n",
+ ret);
+ break;
case HCLGE_MBX_GET_LINK_MODE:
hclge_get_link_mode(vport, req);
break;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 12f16b9..8bc28e6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -620,12 +620,50 @@ static int hclgevf_set_rss_tc_mode(struct hclgevf_dev *hdev, u16 rss_size)
return status;
}
+/* for revision 0x20, vf shared the same rss config with pf */
+static int hclgevf_get_rss_hash_key(struct hclgevf_dev *hdev)
+{
+#define HCLGEVF_RSS_MBX_RESP_LEN 8
+
+ struct hclgevf_rss_cfg *rss_cfg = &hdev->rss_cfg;
+ u8 resp_msg[HCLGEVF_RSS_MBX_RESP_LEN];
+ u16 msg_num, hash_key_index;
+ u8 index;
+ int ret;
+
+ msg_num = (HCLGEVF_RSS_KEY_SIZE + HCLGEVF_RSS_MBX_RESP_LEN - 1) /
+ HCLGEVF_RSS_MBX_RESP_LEN;
+ for (index = 0; index < msg_num; index++) {
+ ret = hclgevf_send_mbx_msg(hdev, HCLGE_MBX_GET_RSS_KEY, 0,
+ &index, sizeof(index),
+ true, resp_msg,
+ HCLGEVF_RSS_MBX_RESP_LEN);
+ if (ret) {
+ dev_err(&hdev->pdev->dev,
+ "VF get rss hash key from PF failed, ret=%d",
+ ret);
+ return ret;
+ }
+
+ hash_key_index = HCLGEVF_RSS_MBX_RESP_LEN * index;
+ if (index == msg_num - 1)
+ memcpy(&rss_cfg->rss_hash_key[hash_key_index],
+ &resp_msg[0],
+ HCLGEVF_RSS_KEY_SIZE - hash_key_index);
+ else
+ memcpy(&rss_cfg->rss_hash_key[hash_key_index],
+ &resp_msg[0], HCLGEVF_RSS_MBX_RESP_LEN);
+ }
+
+ return 0;
+}
+
static int hclgevf_get_rss(struct hnae3_handle *handle, u32 *indir, u8 *key,
u8 *hfunc)
{
struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
struct hclgevf_rss_cfg *rss_cfg = &hdev->rss_cfg;
- int i;
+ int i, ret;
if (handle->pdev->revision >= 0x21) {
/* Get hash algorithm */
@@ -647,6 +685,16 @@ static int hclgevf_get_rss(struct hnae3_handle *handle, u32 *indir, u8 *key,
if (key)
memcpy(key, rss_cfg->rss_hash_key,
HCLGEVF_RSS_KEY_SIZE);
+ } else {
+ if (hfunc)
+ *hfunc = ETH_RSS_HASH_TOP;
+ if (key) {
+ ret = hclgevf_get_rss_hash_key(hdev);
+ if (ret)
+ return ret;
+ memcpy(key, rss_cfg->rss_hash_key,
+ HCLGEVF_RSS_KEY_SIZE);
+ }
}
if (indir)
--
2.7.4
^ permalink raw reply related
* [Patch net-next 09/12] net: hns3: enable VF VLAN filter for each VF when initializing
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Jian Shen, Peng Li, Huazhong Tan
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
From: Jian Shen <shenjian15@huawei.com>
For revision 0x21, the switch of VF VLAN filter is per function.
It's necessary to enable VF VLAN filter for each VF when initializing.
Otherwise, VF will be able to receive broadcast packets with unknown
VLAN when PF enters promisc mode.
Fixes: 64d114f0a750 ("net: hns3: Add egress/ingress vlan filter for revision 0x21")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 4 ++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 30 ++++++++++++++--------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index bad21c89..3714733 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -693,7 +693,9 @@ struct hclge_mac_vlan_remove_cmd {
struct hclge_vlan_filter_ctrl_cmd {
u8 vlan_type;
u8 vlan_fe;
- u8 rsv[22];
+ u8 rsv1[2];
+ u8 vf_id;
+ u8 rsv2[19];
};
struct hclge_vlan_filter_pf_cfg_cmd {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index f113439..deda606 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -6337,7 +6337,7 @@ static int hclge_do_ioctl(struct hnae3_handle *handle, struct ifreq *ifr,
}
static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
- u8 fe_type, bool filter_en)
+ u8 fe_type, bool filter_en, u8 vf_id)
{
struct hclge_vlan_filter_ctrl_cmd *req;
struct hclge_desc desc;
@@ -6348,6 +6348,7 @@ static int hclge_set_vlan_filter_ctrl(struct hclge_dev *hdev, u8 vlan_type,
req = (struct hclge_vlan_filter_ctrl_cmd *)desc.data;
req->vlan_type = vlan_type;
req->vlan_fe = filter_en ? fe_type : 0;
+ req->vf_id = vf_id;
ret = hclge_cmd_send(&hdev->hw, &desc, 1);
if (ret)
@@ -6376,12 +6377,13 @@ static void hclge_enable_vlan_filter(struct hnae3_handle *handle, bool enable)
if (hdev->pdev->revision >= 0x21) {
hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
- HCLGE_FILTER_FE_EGRESS, enable);
+ HCLGE_FILTER_FE_EGRESS, enable, 0);
hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
- HCLGE_FILTER_FE_INGRESS, enable);
+ HCLGE_FILTER_FE_INGRESS, enable, 0);
} else {
hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
- HCLGE_FILTER_FE_EGRESS_V1_B, enable);
+ HCLGE_FILTER_FE_EGRESS_V1_B, enable,
+ 0);
}
if (enable)
handle->netdev_flags |= HNAE3_VLAN_FLTR;
@@ -6689,19 +6691,27 @@ static int hclge_init_vlan_config(struct hclge_dev *hdev)
int i;
if (hdev->pdev->revision >= 0x21) {
- ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
- HCLGE_FILTER_FE_EGRESS, true);
- if (ret)
- return ret;
+ /* for revision 0x21, vf vlan filter is per function */
+ for (i = 0; i < hdev->num_alloc_vport; i++) {
+ vport = &hdev->vport[i];
+ ret = hclge_set_vlan_filter_ctrl(hdev,
+ HCLGE_FILTER_TYPE_VF,
+ HCLGE_FILTER_FE_EGRESS,
+ true,
+ vport->vport_id);
+ if (ret)
+ return ret;
+ }
ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_PORT,
- HCLGE_FILTER_FE_INGRESS, true);
+ HCLGE_FILTER_FE_INGRESS, true,
+ 0);
if (ret)
return ret;
} else {
ret = hclge_set_vlan_filter_ctrl(hdev, HCLGE_FILTER_TYPE_VF,
HCLGE_FILTER_FE_EGRESS_V1_B,
- true);
+ true, 0);
if (ret)
return ret;
}
--
2.7.4
^ permalink raw reply related
* [Patch net-next 12/12] net: hns3: fix improper error handling for hns3_client_start
From: Huazhong Tan @ 2019-02-23 9:22 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
Huazhong Tan, Peng Li
In-Reply-To: <1550913739-13232-1-git-send-email-tanhuazhong@huawei.com>
If hns3_client_start() failed in the hns3_client_init(),
register_dev() should be undo in its error handling.
Fixes: a6d818e31d08 ("net: hns3: Add vport alive state checking support")
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 9f9a52d..3cb43b1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -3656,7 +3656,7 @@ static int hns3_client_init(struct hnae3_handle *handle)
ret = hns3_client_start(handle);
if (ret) {
dev_err(priv->dev, "hns3_client_start fail! ret=%d\n", ret);
- goto out_reg_netdev_fail;
+ goto out_client_start;
}
hns3_dcbnl_setup(handle);
@@ -3670,6 +3670,8 @@ static int hns3_client_init(struct hnae3_handle *handle)
return ret;
+out_client_start:
+ unregister_netdev(netdev);
out_reg_netdev_fail:
hns3_uninit_phy(netdev);
out_init_phy:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Leon Romanovsky @ 2019-02-23 9:26 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <aa883bc5ffd63047cf57e2f9925002e7573fd7ac.1550773362.git.swise@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]
On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
> This function sends the constructed netlink message and then
> receives the response, displaying any error text.
>
> Change 'rdma dev set' to use it.
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> rdma/dev.c | 2 +-
> rdma/rdma.h | 1 +
> rdma/utils.c | 21 +++++++++++++++++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/rdma/dev.c b/rdma/dev.c
> index 60ff4b31e320..d2949c378f08 100644
> --- a/rdma/dev.c
> +++ b/rdma/dev.c
> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
> mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
> mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>
> - return rd_send_msg(rd);
> + return rd_sendrecv_msg(rd, seq);
> }
>
> static int dev_one_set(struct rd *rd)
> diff --git a/rdma/rdma.h b/rdma/rdma.h
> index 547bb5749a39..20be2f12c4f8 100644
> --- a/rdma/rdma.h
> +++ b/rdma/rdma.h
> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
> */
> int rd_send_msg(struct rd *rd);
> int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
> void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
> int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
> int rd_attr_cb(const struct nlattr *attr, void *data);
> diff --git a/rdma/utils.c b/rdma/utils.c
> index 069d44fece10..a6f2826c9605 100644
> --- a/rdma/utils.c
> +++ b/rdma/utils.c
> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
> return ret;
> }
>
> +static int null_cb(const struct nlmsghdr *nlh, void *data)
> +{
> + return MNL_CB_OK;
> +}
> +
> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
> +{
> + int ret;
> +
> + ret = rd_send_msg(rd);
> + if (ret) {
> + perror(NULL);
This is more or less already done in rd_send_msg() and that function
prints something in case of execution error. So the missing piece
is to update rd_recv_msg(), so all places will "magically" print errors
and not only dev_set_name().
> + goto out;
> + }
> + ret = rd_recv_msg(rd, null_cb, rd, seq);
> + if (ret)
> + perror(NULL);
> +out:
> + return ret;
> +}
> +
> static struct dev_map *_dev_map_lookup(struct rd *rd, const char *dev_name)
> {
> struct dev_map *dev_map;
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/4] Sync up rdma_netlink.h
From: Leon Romanovsky @ 2019-02-23 9:28 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <ff4f692bee0d683c0638cf88cbf14faf94ea87af.1550773362.git.swise@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 477 bytes --]
On Thu, Feb 21, 2019 at 08:19:07AM -0800, Steve Wise wrote:
> Pull in the latest rdma_netlink.h to get the RDMA_NLDEV_CMD_NEWLINK /
> RDMA_NLDEV_CMD_DELLINK API.
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> rdma/include/uapi/rdma/rdma_netlink.h | 74 +++++++++++++++++++++++++++--------
> 1 file changed, 58 insertions(+), 16 deletions(-)
>
If my series go first, this patch won't be needed.
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Leon Romanovsky @ 2019-02-23 9:31 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092615.GM23561@mtr-leonro.mtl.com>
[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]
On Sat, Feb 23, 2019 at 11:26:15AM +0200, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
> > This function sends the constructed netlink message and then
> > receives the response, displaying any error text.
> >
> > Change 'rdma dev set' to use it.
> >
> > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > ---
> > rdma/dev.c | 2 +-
> > rdma/rdma.h | 1 +
> > rdma/utils.c | 21 +++++++++++++++++++++
> > 3 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/rdma/dev.c b/rdma/dev.c
> > index 60ff4b31e320..d2949c378f08 100644
> > --- a/rdma/dev.c
> > +++ b/rdma/dev.c
> > @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
> > mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
> > mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
> >
> > - return rd_send_msg(rd);
> > + return rd_sendrecv_msg(rd, seq);
> > }
> >
> > static int dev_one_set(struct rd *rd)
> > diff --git a/rdma/rdma.h b/rdma/rdma.h
> > index 547bb5749a39..20be2f12c4f8 100644
> > --- a/rdma/rdma.h
> > +++ b/rdma/rdma.h
> > @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
> > */
> > int rd_send_msg(struct rd *rd);
> > int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
> > +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
> > void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
> > int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
> > int rd_attr_cb(const struct nlattr *attr, void *data);
> > diff --git a/rdma/utils.c b/rdma/utils.c
> > index 069d44fece10..a6f2826c9605 100644
> > --- a/rdma/utils.c
> > +++ b/rdma/utils.c
> > @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
> > return ret;
> > }
> >
> > +static int null_cb(const struct nlmsghdr *nlh, void *data)
> > +{
> > + return MNL_CB_OK;
> > +}
> > +
> > +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
> > +{
> > + int ret;
> > +
> > + ret = rd_send_msg(rd);
> > + if (ret) {
> > + perror(NULL);
>
> This is more or less already done in rd_send_msg() and that function
> prints something in case of execution error. So the missing piece
> is to update rd_recv_msg(), so all places will "magically" print errors
> and not only dev_set_name().
>
> > + goto out;
> > + }
> > + ret = rd_recv_msg(rd, null_cb, rd, seq);
Will this "null_cb" work for all send/recv flows or only in flows where
response can be error only? Will we need this recv_msg if we implement
extack support?
> > + if (ret)
> > + perror(NULL);
> > +out:
> > + return ret;
> > +}
> > +
> > static struct dev_map *_dev_map_lookup(struct rd *rd, const char *dev_name)
> > {
> > struct dev_map *dev_map;
> > --
> > 1.8.3.1
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 3/4] rdma: add 'link add/delete' commands
From: Leon Romanovsky @ 2019-02-23 9:43 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <5dd9d9aeada48bc5db0eb0394ed4e3ce38ee41bc.1550773362.git.swise@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 3876 bytes --]
On Thu, Feb 21, 2019 at 08:19:12AM -0800, Steve Wise wrote:
> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma
> device to a netdev interface.
>
> EG:
>
> rdma link add rxe_eth0 type rxe netdev eth0
> rdma link delete rxe_eth0
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> rdma/link.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> rdma/rdma.h | 1 +
> rdma/utils.c | 2 +-
> 3 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/rdma/link.c b/rdma/link.c
> index c064be627be2..afaf19663728 100644
> --- a/rdma/link.c
> +++ b/rdma/link.c
> @@ -14,6 +14,9 @@
> static int link_help(struct rd *rd)
> {
> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename);
> + pr_out("Usage: %s link add NAME type TYPE netdev NETDEV\n",
> + rd->filename);
> + pr_out("Usage: %s link delete NAME\n", rd->filename);
> return 0;
> }
>
> @@ -341,10 +344,74 @@ static int link_show(struct rd *rd)
> return rd_exec_link(rd, link_one_show, true);
> }
>
> +static int link_add(struct rd *rd)
> +{
> + char *name;
> + char *type = NULL;
> + char *dev = NULL;
> + uint32_t seq;
> +
> + if (rd_no_arg(rd)) {
> + pr_err("No link name was supplied\n");
I think that it is better to have instruction message and not error
message: "Please provide ...".
> + return -EINVAL;
> + }
> + name = rd_argv(rd);
> + rd_arg_inc(rd);
> + while (!rd_no_arg(rd)) {
> + if (rd_argv_match(rd, "type")) {
> + rd_arg_inc(rd);
> + type = rd_argv(rd);
> + } else if (rd_argv_match(rd, "netdev")) {
> + rd_arg_inc(rd);
> + dev = rd_argv(rd);
> + } else {
> + pr_err("Invalid parameter %s\n", rd_argv(rd));
> + return -EINVAL;
> + }
> + rd_arg_inc(rd);
Please use chains of struct rd_cmd and rd_exec_cmd() instead of
open-coding parser.
> + }
> + if (!type) {
> + pr_err("No type was supplied\n");
> + return -EINVAL;
General parser handle it.
> + }
> + if (!dev) {
> + pr_err("No net device was supplied\n");
> + return -EINVAL;
> + }
rd_exec_require_dev() ???
> +
> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_NEWLINK, &seq,
> + (NLM_F_REQUEST | NLM_F_ACK));
> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, name);
> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_LINK_TYPE, type);
> + mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_NDEV_NAME, dev);
> + return rd_sendrecv_msg(rd, seq);
> +}
> +
> +static int _link_del(struct rd *rd)
> +{
> + uint32_t seq;
> +
> + if (!rd_no_arg(rd)) {
> + pr_err("Unknown parameter %s\n", rd_argv(rd));
> + return -EINVAL;
> + }
> + rd_prepare_msg(rd, RDMA_NLDEV_CMD_DELLINK, &seq,
> + (NLM_F_REQUEST | NLM_F_ACK));
> + mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
> + return rd_sendrecv_msg(rd, seq);
> +}
> +
> +static int link_del(struct rd *rd)
> +{
> + return rd_exec_require_dev(rd, _link_del);
> +}
> +
> int cmd_link(struct rd *rd)
> {
> const struct rd_cmd cmds[] = {
> { NULL, link_show },
> + { "add", link_add },
> + { "delete", link_del },
> { "show", link_show },
> { "list", link_show },
> { "help", link_help },
> diff --git a/rdma/rdma.h b/rdma/rdma.h
> index 20be2f12c4f8..af4597f45640 100644
> --- a/rdma/rdma.h
> +++ b/rdma/rdma.h
> @@ -79,6 +79,7 @@ struct rd_cmd {
> */
> bool rd_no_arg(struct rd *rd);
> void rd_arg_inc(struct rd *rd);
> +bool rd_argv_match(struct rd *rd, const char *pattern);
>
> char *rd_argv(struct rd *rd);
>
> diff --git a/rdma/utils.c b/rdma/utils.c
> index a6f2826c9605..85a954167511 100644
> --- a/rdma/utils.c
> +++ b/rdma/utils.c
> @@ -32,7 +32,7 @@ int strcmpx(const char *str1, const char *str2)
> return strncmp(str1, str2, strlen(str1));
> }
>
> -static bool rd_argv_match(struct rd *rd, const char *pattern)
> +bool rd_argv_match(struct rd *rd, const char *pattern)
> {
> if (!rd_argc(rd))
> return false;
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Ido Schimmel @ 2019-02-23 10:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190222235927.10295-8-f.fainelli@gmail.com>
On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:
> Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
> from all clients, which were migrated to use switchdev notification in
> the previous patches.
>
> Add a new function switchdev_port_attr_notify() that sends the switchdev
> notifications SWITCHDEV_PORT_ATTR_SET and takes care, depending on
> SWITCHDEV_F_DEFER to call the blocking (process) or non-blocking
> (atomic) notifier chain accordingly.
>
> Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
> likewise.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/switchdev/switchdev.c | 96 +++++++++++----------------------------
> 1 file changed, 26 insertions(+), 70 deletions(-)
>
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 94400f5b8e07..a1f16836ef89 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -174,81 +174,35 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
> return 0;
> }
>
> -/**
> - * switchdev_port_attr_get - Get port attribute
Hmm, why do you remove it here? Can't you remove it in a separate patch?
I thought we already got rid of it :p
> - *
> - * @dev: port device
> - * @attr: attribute to get
> - */
> -int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
> +static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
> + struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans)
> {
> - const struct switchdev_ops *ops = dev->switchdev_ops;
> - struct net_device *lower_dev;
> - struct list_head *iter;
> - struct switchdev_attr first = {
> - .id = SWITCHDEV_ATTR_ID_UNDEFINED
> - };
> - int err = -EOPNOTSUPP;
> + int err;
> + int rc;
>
> - if (ops && ops->switchdev_port_attr_get)
> - return ops->switchdev_port_attr_get(dev, attr);
> + struct switchdev_notifier_port_attr_info attr_info = {
> + .attr = attr,
> + .trans = trans,
> + .handled = false,
> + };
>
> - if (attr->flags & SWITCHDEV_F_NO_RECURSE)
> + if (attr & SWITCHDEV_F_DEFER)
> + rc = call_switchdev_blocking_notifiers(nt, dev,
> + &attr_info.info, NULL);
> + else
> + rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
I don't believe this is needed. You're calling this function from
switchdev_port_attr_set_now() which is always called from process
context. switchdev_port_attr_set() takes care of that. Similar to
switchdev_port_obj_add().
The event `SWITCHDEV_PORT_ATTR_SET` is therefore always blocking and
drivers only need to take care of it from their blocking notifier.
> + err = notifier_to_errno(rc);
> + if (err) {
> + WARN_ON(!attr_info.handled);
> return err;
> -
> - /* Switch device port(s) may be stacked under
> - * bond/team/vlan dev, so recurse down to get attr on
> - * each port. Return -ENODATA if attr values don't
> - * compare across ports.
> - */
> -
> - netdev_for_each_lower_dev(dev, lower_dev, iter) {
> - err = switchdev_port_attr_get(lower_dev, attr);
> - if (err)
> - break;
> - if (first.id == SWITCHDEV_ATTR_ID_UNDEFINED)
> - first = *attr;
> - else if (memcmp(&first, attr, sizeof(*attr)))
> - return -ENODATA;
> }
>
> - return err;
> -}
> -EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
> -
> -static int __switchdev_port_attr_set(struct net_device *dev,
> - const struct switchdev_attr *attr,
> - struct switchdev_trans *trans)
> -{
> - const struct switchdev_ops *ops = dev->switchdev_ops;
> - struct net_device *lower_dev;
> - struct list_head *iter;
> - int err = -EOPNOTSUPP;
> -
> - if (ops && ops->switchdev_port_attr_set) {
> - err = ops->switchdev_port_attr_set(dev, attr, trans);
> - goto done;
> - }
> -
> - if (attr->flags & SWITCHDEV_F_NO_RECURSE)
> - goto done;
> -
> - /* Switch device port(s) may be stacked under
> - * bond/team/vlan dev, so recurse down to set attr on
> - * each port.
> - */
> -
> - netdev_for_each_lower_dev(dev, lower_dev, iter) {
> - err = __switchdev_port_attr_set(lower_dev, attr, trans);
> - if (err)
> - break;
> - }
> -
> -done:
> - if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
> - err = 0;
> + if (!attr_info.handled)
> + return -EOPNOTSUPP;
>
> - return err;
> + return 0;
> }
>
> static int switchdev_port_attr_set_now(struct net_device *dev,
> @@ -267,7 +221,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
> */
>
> trans.ph_prepare = true;
> - err = __switchdev_port_attr_set(dev, attr, &trans);
> + err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
> + &trans);
> if (err) {
> /* Prepare phase failed: abort the transaction. Any
> * resources reserved in the prepare phase are
> @@ -286,7 +241,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
> */
>
> trans.ph_prepare = false;
> - err = __switchdev_port_attr_set(dev, attr, &trans);
> + err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
> + &trans);
> WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
> dev->name, attr->id);
> switchdev_trans_items_warn_destroy(dev, &trans);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next 1/8] switchdev: Add SWITCHDEV_PORT_ATTR_SET
From: Ido Schimmel @ 2019-02-23 10:33 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190222235927.10295-2-f.fainelli@gmail.com>
On Fri, Feb 22, 2019 at 03:59:19PM -0800, Florian Fainelli wrote:
> In preparation for allowing switchdev enabled drivers to veto specific
> attribute settings from within the context of the caller, introduce a
> new switchdev notifier type for port attributes.
>
> Suggested-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> include/net/switchdev.h | 27 +++++++++++++++++++++
> net/switchdev/switchdev.c | 51 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index 45310ddf2d7e..ca56b7487540 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -136,6 +136,7 @@ enum switchdev_notifier_type {
>
> SWITCHDEV_PORT_OBJ_ADD, /* Blocking. */
> SWITCHDEV_PORT_OBJ_DEL, /* Blocking. */
> + SWITCHDEV_PORT_ATTR_SET, /* May be blocking . */
See my comment on 7/8. IIUC, this is always blocking, so comment needs to
be changed.
>
> SWITCHDEV_VXLAN_FDB_ADD_TO_BRIDGE,
> SWITCHDEV_VXLAN_FDB_DEL_TO_BRIDGE,
> @@ -164,6 +165,13 @@ struct switchdev_notifier_port_obj_info {
> bool handled;
> };
>
> +struct switchdev_notifier_port_attr_info {
> + struct switchdev_notifier_info info; /* must be first */
> + const struct switchdev_attr *attr;
> + struct switchdev_trans *trans;
> + bool handled;
> +};
> +
> static inline struct net_device *
> switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
> {
> @@ -216,7 +224,15 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
> int (*del_cb)(struct net_device *dev,
> const struct switchdev_obj *obj));
>
> +int switchdev_handle_port_attr_set(struct net_device *dev,
> + struct switchdev_notifier_port_attr_info *port_attr_info,
> + bool (*check_cb)(const struct net_device *dev),
> + int (*set_cb)(struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans));
> +
> #define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
> +
> #else
>
> static inline void switchdev_deferred_process(void)
> @@ -303,6 +319,17 @@ switchdev_handle_port_obj_del(struct net_device *dev,
> return 0;
> }
>
> +static inline int
> +switchdev_handle_port_attr_set(struct net_device *dev,
> + struct switchdev_notifier_port_attr_info *port_attr_info,
> + bool (*check_cb)(const struct net_device *dev),
> + int (*set_cb)(struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans))
> +{
> + return 0;
> +}
> +
> #define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
>
> #endif
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 7e1357db33d7..94400f5b8e07 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -697,3 +697,54 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
> return err;
> }
> EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_del);
> +
> +static int __switchdev_handle_port_attr_set(struct net_device *dev,
> + struct switchdev_notifier_port_attr_info *port_attr_info,
> + bool (*check_cb)(const struct net_device *dev),
> + int (*set_cb)(struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans))
> +{
> + struct net_device *lower_dev;
> + struct list_head *iter;
> + int err = -EOPNOTSUPP;
> +
> + if (check_cb(dev)) {
> + port_attr_info->handled = true;
> + return set_cb(dev, port_attr_info->attr,
> + port_attr_info->trans);
> + }
> +
> + /* Switch ports might be stacked under e.g. a LAG. Ignore the
> + * unsupported devices, another driver might be able to handle them. But
> + * propagate to the callers any hard errors.
> + *
> + * If the driver does its own bookkeeping of stacked ports, it's not
> + * necessary to go through this helper.
> + */
> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
> + err = __switchdev_handle_port_attr_set(lower_dev, port_attr_info,
> + check_cb, set_cb);
> + if (err && err != -EOPNOTSUPP)
> + return err;
> + }
> +
> + return err;
> +}
> +
> +int switchdev_handle_port_attr_set(struct net_device *dev,
> + struct switchdev_notifier_port_attr_info *port_attr_info,
> + bool (*check_cb)(const struct net_device *dev),
> + int (*set_cb)(struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans))
> +{
> + int err;
> +
> + err = __switchdev_handle_port_attr_set(dev, port_attr_info, check_cb,
> + set_cb);
> + if (err == -EOPNOTSUPP)
> + err = 0;
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(switchdev_handle_port_attr_set);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next 4/8] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
From: Ido Schimmel @ 2019-02-23 10:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190222235927.10295-5-f.fainelli@gmail.com>
On Fri, Feb 22, 2019 at 03:59:22PM -0800, Florian Fainelli wrote:
> Following patches will change the way we communicate setting a port's
> attribute and use a notifier to perform those tasks.
>
> Prepare mlxsw to support receiving notifier events targeting
> SWITCHDEV_PORT_ATTR_SET and utilize the switchdev_handle_port_attr_set()
> to handle stacking of devices.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index 766f5b5f1cf5..c1aedfea3a31 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -3123,6 +3123,13 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
> struct net_device *br_dev;
> int err;
>
> + if (event == SWITCHDEV_PORT_ATTR_SET) {
> + err = switchdev_handle_port_attr_set(dev, ptr,
> + mlxsw_sp_port_dev_check,
> + mlxsw_sp_port_attr_set);
> + return notifier_from_errno(err);
I don't think this code is ever executed. And if it was executed, we
would have problems because switchdev_handle_port_attr_set() might
block.
> + }
> +
> /* Tunnel devices are not our uppers, so check their master instead */
> br_dev = netdev_master_upper_dev_get_rcu(dev);
> if (!br_dev)
> @@ -3446,6 +3453,11 @@ static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
> mlxsw_sp_port_dev_check,
> mlxsw_sp_port_obj_del);
> return notifier_from_errno(err);
> + case SWITCHDEV_PORT_ATTR_SET:
> + err = switchdev_handle_port_attr_set(dev, ptr,
> + mlxsw_sp_port_dev_check,
> + mlxsw_sp_port_attr_set);
> + return notifier_from_errno(err);
> }
>
> return NOTIFY_DONE;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Jesper Dangaard Brouer @ 2019-02-23 10:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Toke Høiland-Jørgensen, David Miller, netdev,
Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190222133734.1880a88d@cakuba.netronome.com>
On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
[...]
> > >
> > > BPF programs don't obey by netns boundaries. The fact the program is
> > > verified in one ns doesn't mean this is the only ns it will be used in :(
> > > Meaning if any program is using the redirect map you may need a secret
> > > map in every ns.. no?
> >
> > Ah, yes, good point. Totally didn't think about the fact that load and
> > attach are decoupled. Hmm, guess I'll just have to move the call to
> > alloc_default_map() to the point where the program is attached to an
> > interface, then...
>
> Possibly.. and you also need to handle the case where interface with a
> program attached is moved, no?
True, we need to handle if e.g. a veth gets an XDP program attached and
then is moved into a network namespace (as I've already explained to
Toke in a meeting).
I'm still not sure how to handle this...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH] net: ax25: Add SPDX License to fix missing License warning
From: Jules Irenge @ 2019-02-23 11:11 UTC (permalink / raw)
To: jreuter
Cc: linux-hams, linux-kernel, davem, ralf, netdev, outreachy-kernel,
gregkh, jbi.octave
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
net/ax25/af_ax25.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 5d01edf8d819..5048790a75f6 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
--
2.20.1
^ permalink raw reply related
* [PATCH] net: ax25: Fix multiple blank lines warning
From: Jules Irenge @ 2019-02-23 11:17 UTC (permalink / raw)
To: jreuter
Cc: linux-hams, linux-kernel, davem, ralf, netdev, outreachy-kernel,
jbi.octave
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
net/ax25/af_ax25.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 5048790a75f6..2da349b7854a 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -49,8 +49,6 @@
#include <net/ip.h>
#include <net/arp.h>
-
-
HLIST_HEAD(ax25_list);
DEFINE_SPINLOCK(ax25_list_lock);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-23 11:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190222133029.131d066d@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Fri, 22 Feb 2019 10:47:10 +0100, Toke Høiland-Jørgensen wrote:
>> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>>
>> > On Fri, 22 Feb 2019 00:02:23 +0100, Toke Høiland-Jørgensen wrote:
>> >> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> >>
>> >> > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> >> >> A common pattern when using xdp_redirect_map() is to create a device map
>> >> >> where the lookup key is simply ifindex. Because device maps are arrays,
>> >> >> this leaves holes in the map, and the map has to be sized to fit the
>> >> >> largest ifindex, regardless of how many devices actually are actually
>> >> >> needed in the map.
>> >> >>
>> >> >> This patch adds a second type of device map where the key is interpreted as
>> >> >> an ifindex and looked up using a hashmap, instead of being used as an array
>> >> >> index. This leads to maps being densely packed, so they can be smaller.
>> >> >>
>> >> >> The default maps used by xdp_redirect() are changed to use the new map
>> >> >> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
>> >> >> but instead to 64 total simultaneous interfaces per network namespace. This
>> >> >> also provides an easy way to compare the performance of devmap and
>> >> >> devmap_idx:
>> >> >>
>> >> >> xdp_redirect_map (devmap): 8394560 pkt/s
>> >> >> xdp_redirect (devmap_idx): 8179480 pkt/s
>> >> >>
>> >> >> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
>> >> >
>> >> > Could you share what the ifindex mix was here, to arrive at these
>> >> > numbers? How does it compare to using an array but not keying with
>> >> > ifindex?
>> >>
>> >> Just the standard set on my test machine; ifindex 1 through 9, except 8
>> >> in this case. So certainly no more than 1 ifindex in each hash bucket
>> >> for those numbers.
>> >
>> > Oh, I clearly misread your numbers, it's still slower than array, you
>> > just don't need the size limit.
>>
>> Yeah, this is not about speeding up devmap, it's about lifting the size
>> restriction.
>>
>> >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> >> >
>> >> >> +static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
>> >> >> + u64 map_flags)
>> >> >> +{
>> >> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
>> >> >> + struct bpf_dtab_netdev *dev, *old_dev;
>> >> >> + u32 idx = *(u32 *)key;
>> >> >> + u32 val = *(u32 *)value;
>> >> >> + u32 bit;
>> >> >> +
>> >> >> + if (unlikely(map_flags > BPF_EXIST))
>> >> >> + return -EINVAL;
>> >> >> + if (unlikely(map_flags == BPF_NOEXIST))
>> >> >> + return -EEXIST;
>> >> >> +
>> >> >> + old_dev = __dev_map_idx_lookup_elem(map, idx);
>> >> >> + if (!val) {
>> >> >> + if (!old_dev)
>> >> >> + return 0;
>> >> >
>> >> > IMHO this is a fairly strange mix of array and hashmap semantics. I
>> >> > think you should stick to hashmap behaviour AFA flags and
>> >> > update/delete goes.
>> >>
>> >> Yeah, the double book-keeping is a bit strange, but it allows the actual
>> >> forwarding and flush code to be reused between both types of maps. I
>> >> think this is worth the slight semantic confusion :)
>> >
>> > I'm not sure I was clear, let me try again :) Your get_next_key only
>> > reports existing indexes if I read the code right, so that's not an
>> > array - in an array indexes always exist. What follows inserting 0
>> > should not be equivalent to delete and BPF_NOEXIST should be handled
>> > appropriately.
>>
>> Ah, I see what you mean. Yeah, sure, I guess I can restrict deletion to
>> only working through explicit delete.
>>
>> I could also add a fail on NOEXIST, but since each index is tied to a
>> particular value, you can't actually change the contents of each index,
>> only insert and remove. So why would you ever set that flag?
>
> The reason user would have for setting the flag is not clear :) But
> if you want to reject it because it's unsupported/makes no sense, you
> should do EINVAL, not EEXIST ;)
>
>> > Different maps behave differently, I think it's worth trying to limit
>> > the divergence in how things behave to the basic array and a hashmap
>> > models when possible.
>>
>> So I don't actually think of this as a hashmap in the general sense;
>> after all, you can only store ifindexes in it, and key and value are
>> tied to one another. So it's an ifindex'ed devmap (which is also why I
>> named it devmap_idx and not devmap_hash); the fact that it's implemented
>> as a hashmap is just incidental.
>>
>> So I guess it's a choice between being consistent with the other devmap
>> type, or with a general hashmap. I'm not actually sure that the latter
>> is less surprising? :)
>
> The distinction is that if entry is not in the map get_next won't
> return its key. As you say the construct is not really a hash map
> (probably a set is the closest) but it most definitely is not an
> array, so no hard EEXIST on NOEXIST flag :)
Yeah, I thought about it some more and I agree it makes sense to change
the update semantics to be a bit more hashmap-like :)
-Toke
^ permalink raw reply
* [PATCH] tun: fix blocking read
From: Timur Celik @ 2019-02-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Timur Celik
This patch moves setting of the current state into the loop. Otherwise
the task may end up in a busy wait loop if none of the break conditions
are met.
Signed-off-by: Timur Celik <mail@timurcelik.de>
---
drivers/net/tun.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fed298c0cb39..d291762b9e9d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2167,9 +2167,9 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
}
add_wait_queue(&tfile->wq.wait, &wait);
- current->state = TASK_INTERRUPTIBLE;
while (1) {
+ set_current_state(TASK_INTERRUPTIBLE);
ptr = ptr_ring_consume(&tfile->tx_ring);
if (ptr)
break;
@@ -2185,7 +2185,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
schedule();
}
- current->state = TASK_RUNNING;
+ set_current_state(TASK_RUNNING);
remove_wait_queue(&tfile->wq.wait, &wait);
out:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-23 12:11 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jakub Kicinski
Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov, brouer
In-Reply-To: <20190223114343.5813f18a@carbon>
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
>> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
>> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
> [...]
>> > >
>> > > BPF programs don't obey by netns boundaries. The fact the program is
>> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> > > Meaning if any program is using the redirect map you may need a secret
>> > > map in every ns.. no?
>> >
>> > Ah, yes, good point. Totally didn't think about the fact that load and
>> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> > alloc_default_map() to the point where the program is attached to an
>> > interface, then...
>>
>> Possibly.. and you also need to handle the case where interface with a
>> program attached is moved, no?
Yup, alloc on attach was easy enough; the moving turns out to be the
tricky part :)
> True, we need to handle if e.g. a veth gets an XDP program attached and
> then is moved into a network namespace (as I've already explained to
> Toke in a meeting).
Yeah, I had somehow convinced myself that the XDP program was being
removed when the interface was being torn down before moving between
namespaces. Jesper pointed out that this was not in fact the case... :P
> I'm still not sure how to handle this...
There are a couple of options, I think. At least:
1. Maintain a flag on struct net_device indicating that this device
needs the redirect map allocated, and react to that when interfaces
are being moved.
2. Lookup the BPF program by ID (which we can get from the driver) on
move, and react to the program flag.
3. Keep the allocation on program load, but allocate maps for all active
namespaces (which would probably need a refcnt mechanism to
deallocate things again).
I think I'm leaning towards #2; possibly combined with a refcnt so we
can actually deallocate the map in the root namespace when it's not
needed anymore.
-Toke
^ permalink raw reply
* Re: [PATCH] net: ax25: Add SPDX License to fix missing License warning
From: Greg KH @ 2019-02-23 12:58 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111100.13983-1-jbi.octave@gmail.com>
On Sat, Feb 23, 2019 at 11:11:00AM +0000, Jules Irenge wrote:
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
I know I never take patches without any changelog text :(
> ---
> net/ax25/af_ax25.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5d01edf8d819..5048790a75f6 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
That is the incorrect license of this file!
NEVER add or modify a SPDX line if you do not really really really know
what you are doing. Putting the wrong license tag on a file is worse
than not having any tag at all.
BE VERY CAREFUL ABOUT THESE THINGS, they matter. If you get them wrong,
lawyers get very grumpy :)
thanks,
greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Greg KH @ 2019-02-23 12:59 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111751.14620-1-jbi.octave@gmail.com>
On Sat, Feb 23, 2019 at 11:17:51AM +0000, Jules Irenge wrote:
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> net/ax25/af_ax25.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5048790a75f6..2da349b7854a 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -49,8 +49,6 @@
> #include <net/ip.h>
> #include <net/arp.h>
>
> -
> -
> HLIST_HEAD(ax25_list);
> DEFINE_SPINLOCK(ax25_list_lock);
Why are you doing outreachy application patches in net/? We only accept
them in drivers/staging/ Always start there first.
thanks,
greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Ivan Ivanov @ 2019-02-23 13:17 UTC (permalink / raw)
To: Jules Irenge, Greg KH, jreuter, linux-hams,
Linux Kernel Mailing List, ralf, davem, netdev, outreachy-kernel
In-Reply-To: <20190223125932.GC16686@kroah.com>
as usual, Outreachy is doing stiuupid almost-useless commits like
remove two blank lines or fix a tiny typo. They are trying hard to
pretend that they are useful by doing many lame patches like this,
while promoting their SJW madness, anti-male sieexism and anti-white
riaacism. But with the lame stuff like this patch, they are failing
miserably. Maybe hire a ghostwriter (from NSA) to write some drivers
for you?
P.S. and no I am not going to write them for you because I disagree
with your Ci0ck of Conduct and now have to write from anonymous
account
On Sat, Feb 23, 2019, Greg KH <greg@kroah.com> wrote:
>
> On Sat, Feb 23, 2019 at 11:17:51AM +0000, Jules Irenge wrote:
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> > net/ax25/af_ax25.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> > index 5048790a75f6..2da349b7854a 100644
> > --- a/net/ax25/af_ax25.c
> > +++ b/net/ax25/af_ax25.c
> > @@ -49,8 +49,6 @@
> > #include <net/ip.h>
> > #include <net/arp.h>
> >
> > -
> > -
> > HLIST_HEAD(ax25_list);
> > DEFINE_SPINLOCK(ax25_list_lock);
>
> Why are you doing outreachy application patches in net/? We only accept
> them in drivers/staging/ Always start there first.
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: ax25: Fix multiple blank lines warning
From: Julia Lawall @ 2019-02-23 13:22 UTC (permalink / raw)
To: Jules Irenge
Cc: jreuter, linux-hams, linux-kernel, davem, ralf, netdev,
outreachy-kernel
In-Reply-To: <20190223111751.14620-1-jbi.octave@gmail.com>
On Sat, 23 Feb 2019, Jules Irenge wrote:
You need a commit message here.
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> net/ax25/af_ax25.c | 2 --
This is the same file as in your previous patch, so they should be in a
series.
But please work on files in drivers/staging instead.
julia
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 5048790a75f6..2da349b7854a 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -49,8 +49,6 @@
> #include <net/ip.h>
> #include <net/arp.h>
>
> -
> -
> HLIST_HEAD(ax25_list);
> DEFINE_SPINLOCK(ax25_list_lock);
>
> --
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190223111751.14620-1-jbi.octave%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox