DPDK patches and discussions
 help / color / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
	Igor Russkikh <igor.russkikh@aquantia.com>,
	Pavel Belous <pavel.belous@aquantia.com>,
	Ravi Kumar <ravi1.kumar@amd.com>, Rasesh Mody <rmody@marvell.com>,
	Shahed Shaikh <shshaikh@marvell.com>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	Chas Williams <chas3@att.com>,
	Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	Sachin Saxena <sachin.saxena@nxp.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Gagandeep Singh <g.singh@nxp.com>,
	John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	Gaetan Rivet <gaetan.rivet@6wind.com>,
	Qi Zhang <qi.z.zhang@intel.com>,
	Xiao Wang <xiao.w.wang@intel.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Rosen Xu <rosen.xu@intel.com>,
	Konstantin Ananyev <konstantin.ananyev@intel.com>,
	Shijith Thotton <sthotton@marvell.com>,
	Srisivasubramanian Srinivasan <srinivasan@marvell.com>,
	Matan Azrad <matan@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Yongseok Koh <yskoh@mellanox.com>,
	Viacheslav Ovsiienko <viacheslavo@mellanox.com>,
	Tomasz Duszynski <tdu@semihalf.com>,
	Liron Himi <lironh@marvell.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Rastislav Cernay <cernay@netcope.com>,
	Jan Remes <remes@netcope.com>, Jerin Jacob <jerinj@marvell.com>,
	Nithin Dabilpuram <ndabilpuram@marvell.com>,
	Kiran Kumar K <kirankumark@marvell.com>,
	Keith Wiles <keith.wiles@intel.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	Tiwei Bie <tiwei.bie@intel.com>,
	Zhihong Wang <zhihong.wang@intel.com>,
	Yong Wang <yongwang@vmware.com>,
	Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
Subject: Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status
Date: Tue, 24 Sep 2019 09:27:03 +0100
Message-ID: <edff58b0-339d-f51b-5ef4-dccfb9eecc0b@intel.com> (raw)
In-Reply-To: <1568031190-16510-5-git-send-email-arybchenko@solarflare.com>

On 9/9/2019 1:13 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> 
> Enabling/disabling of allmulticast 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: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> @@ -1227,23 +1227,27 @@ atl_dev_promiscuous_disable(struct rte_eth_dev *dev)
>  	return 0;
>  }
>  
> -static void
> +static int
>  atl_dev_allmulticast_enable(struct rte_eth_dev *dev)
>  {
>  	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  
>  	hw_atl_rpfl2_accept_all_mc_packets_set(hw, true);
> +
> +	return 0;
>  }
>  
> -static void
> +static int
>  atl_dev_allmulticast_disable(struct rte_eth_dev *dev)
>  {
>  	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  
>  	if (dev->data->promiscuous == 1)
> -		return; /* must remain in all_multicast mode */
> +		return 0; /* must remain in all_multicast mode */

What about making this change in the API level, so all dev_ops not need to do
the similar check.
Indeed I can see you are already adding this to API in following patches, but we
can document and guarantee this behavior in API level, so driver developers can
know and rely on this behavior, what do you think?

And this can be a general guideline for all enable/disable APIs. If the feature
already enabled, detect in the API and don't call underlying enable dev_ops,
same for the disable. This saves doing the checks by multiple drivers.

<...>

> @@ -58,10 +58,10 @@ typedef int (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev);
>  typedef int (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev);
>  /**< @internal Function used to disable the RX promiscuous mode of an Ethernet device. */
>  
> -typedef void (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev);
> +typedef int (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev);
>  /**< @internal Enable the receipt of all multicast packets by an Ethernet device. */
>  
> -typedef void (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev);
> +typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev);
>  /**< @internal Disable the receipt of all multicast packets by an Ethernet device. */

Can you please document what a driver should return? As done in other patches.

  parent reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls " Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch Andrew Rybchenko
2019-09-09 12:56   ` Gaëtan Rivet
2019-09-13 21:04     ` Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 3/7] net/bonding: " Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko
2019-09-16  7:03   ` Hyong Youb Kim (hyonkim)
2019-09-16  7:29     ` Andrew Rybchenko
2019-09-24  8:27   ` Ferruh Yigit [this message]
2019-09-24 12:14     ` Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again Andrew Rybchenko
2019-09-24  8:36   ` Ferruh Yigit
2019-09-24  8:54     ` Andrew Rybchenko
2019-09-24 11:03       ` Ferruh Yigit
2019-09-24 11:50         ` Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 6/7] app/testpmd: check code of allmulticast mode switch Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 7/7] examples/ipv4_multicast: check allmulticast enable status Andrew Rybchenko

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edff58b0-339d-f51b-5ef4-dccfb9eecc0b@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=Ivan.Ilchenko@oktetlabs.ru \
    --cc=ajit.khaparde@broadcom.com \
    --cc=arybchenko@solarflare.com \
    --cc=beilei.xing@intel.com \
    --cc=cernay@netcope.com \
    --cc=chas3@att.com \
    --cc=dev@dpdk.org \
    --cc=g.singh@nxp.com \
    --cc=gaetan.rivet@6wind.com \
    --cc=haiyangz@microsoft.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=hyonkim@cisco.com \
    --cc=igor.russkikh@aquantia.com \
    --cc=jerinj@marvell.com \
    --cc=jingjing.wu@intel.com \
    --cc=johndale@cisco.com \
    --cc=keith.wiles@intel.com \
    --cc=kirankumark@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=kys@microsoft.com \
    --cc=lironh@marvell.com \
    --cc=matan@mellanox.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=ndabilpuram@marvell.com \
    --cc=pavel.belous@aquantia.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=rahul.lakkireddy@chelsio.com \
    --cc=ravi1.kumar@amd.com \
    --cc=remes@netcope.com \
    --cc=rmody@marvell.com \
    --cc=rosen.xu@intel.com \
    --cc=sachin.saxena@nxp.com \
    --cc=shahafs@mellanox.com \
    --cc=shshaikh@marvell.com \
    --cc=somnath.kotur@broadcom.com \
    --cc=srinivasan@marvell.com \
    --cc=sthemmin@microsoft.com \
    --cc=sthotton@marvell.com \
    --cc=tdu@semihalf.com \
    --cc=thomas@monjalon.net \
    --cc=tiwei.bie@intel.com \
    --cc=viacheslavo@mellanox.com \
    --cc=wenzhuo.lu@intel.com \
    --cc=xiao.w.wang@intel.com \
    --cc=yongwang@vmware.com \
    --cc=yskoh@mellanox.com \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox