> -----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.
>
> 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.
-Hyong
