On 9/11/19 11:46 AM, Hyong Youb Kim (hyonkim) wrote:
-----Original Message-----
From: Andrew Rybchenko <arybche...@solarflare.com>
Sent: Monday, September 9, 2019 8:59 PM
[...]
Subject: [PATCH v2 04/13] ethdev: change promiscuous callbacks to return
status
Enabling/disabling of promiscuous mode is not always successful and
it should be taken into account to be able to handle it properly.
When correct return status is unclear from driver code, -EAGAIN is used.
Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
---
[...]
drivers/net/enic/enic.h | 2 +-
drivers/net/enic/enic_ethdev.c | 22 +++++++---
drivers/net/enic/enic_main.c | 4 +-
[...]
static void
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 5a92508f00..72b1e7956b 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -305,7 +305,7 @@ int enic_get_link_status(struct enic *enic);
int enic_dev_stats_get(struct enic *enic,
struct rte_eth_stats *r_stats);
void enic_dev_stats_clear(struct enic *enic);
-void enic_add_packet_filter(struct enic *enic);
+int enic_add_packet_filter(struct enic *enic);
int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
int enic_del_mac_address(struct enic *enic, int mac_index);
unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 90fdeda901..5d48930a9d 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -603,29 +603,39 @@ static const uint32_t
*enicpmd_dev_supported_ptypes_get(struct rte_eth_dev *dev)
return NULL;
}
-static void enicpmd_dev_promiscuous_enable(struct rte_eth_dev
*eth_dev)
+static int enicpmd_dev_promiscuous_enable(struct rte_eth_dev *eth_dev)
{
struct enic *enic = pmd_priv(eth_dev);
+ int ret;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
- return;
+ return -ENOTSUP;
Should return -E_RTE_SECONDARY to be consistent with other handlers
that check primary/secondary.
I'll fix in the next version, but please, note that -ENOTSUP has
special handling in ethdev patch on config restore. It looks like
different error code should not be a problem here, but please, check.
ENICPMD_FUNC_TRACE();
enic->promisc = 1;
- enic_add_packet_filter(enic);
+ ret = enic_add_packet_filter(enic);
+ if (ret != 0)
+ enic->promisc = 0;
+
+ return ret;
}
-static void enicpmd_dev_promiscuous_disable(struct rte_eth_dev
*eth_dev)
+static int enicpmd_dev_promiscuous_disable(struct rte_eth_dev *eth_dev)
{
struct enic *enic = pmd_priv(eth_dev);
+ int ret;
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
- return;
+ return -ENOTSUP;
Should return -E_RTE_SECONDARY here too.
ENICPMD_FUNC_TRACE();
enic->promisc = 0;
- enic_add_packet_filter(enic);
+ ret = enic_add_packet_filter(enic);
+ if (ret != 0)
+ enic->promisc = 1;
+
+ return ret;
}
static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 40af3781b3..f4e76a057a 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1364,10 +1364,10 @@ int enic_set_vlan_strip(struct enic *enic)
enic->rss_enable);
}
-void enic_add_packet_filter(struct enic *enic)
+int enic_add_packet_filter(struct enic *enic)
{
/* Args -> directed, multicast, broadcast, promisc, allmulti */
- vnic_dev_packet_filter(enic->vdev, 1, 1, 1,
+ return vnic_dev_packet_filter(enic->vdev, 1, 1, 1,
enic->promisc, enic->allmulti);
}
A couple minor comments above. Other than those, patch works fine for enic.
Feel free to add my ack on v2..
Acked-by: Hyong Youb Kim <hyon...@cisco.com>
Thanks for review,
Andrew.
