DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Chen, Jing D" <jing.d.chen@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/6] ether: enhancement for VMDQ support
Date: Wed, 15 Oct 2014 10:10:58 +0200
Message-ID: <10317086.OmtuOpvoDk@xps13> (raw)
In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D015F35FE@shsmsx102.ccr.corp.intel.com>

2014-10-15 06:59, Chen, Jing D:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > >  enum rte_eth_rx_mq_mode {
> > > -	ETH_MQ_RX_NONE = 0,  /**< None of DCB,RSS or VMDQ mode */
> > > -
> > > -	ETH_MQ_RX_RSS,       /**< For RX side, only RSS is on */
> > > -	ETH_MQ_RX_DCB,       /**< For RX side,only DCB is on. */
> > > -	ETH_MQ_RX_DCB_RSS,   /**< Both DCB and RSS enable */
> > > -
> > > -	ETH_MQ_RX_VMDQ_ONLY, /**< Only VMDQ, no RSS nor DCB */
> > > -	ETH_MQ_RX_VMDQ_RSS,  /**< RSS mode with VMDQ */
> > > -	ETH_MQ_RX_VMDQ_DCB,  /**< Use VMDQ+DCB to route traffic to
> > queues */
> > > -	ETH_MQ_RX_VMDQ_DCB_RSS, /**< Enable both VMDQ and DCB in
> > VMDq */
> > > +	/**< None of DCB,RSS or VMDQ mode */
> > > +	ETH_MQ_RX_NONE = 0,
> > > +
> > > +	/**< For RX side, only RSS is on */
> > > +	ETH_MQ_RX_RSS = ETH_MQ_RX_RSS_FLAG,
> > > +	/**< For RX side,only DCB is on. */
> > > +	ETH_MQ_RX_DCB = ETH_MQ_RX_DCB_FLAG,
> > > +	/**< Both DCB and RSS enable */
> > > +	ETH_MQ_RX_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> > ETH_MQ_RX_DCB_FLAG,
> > > +
> > > +	/**< Only VMDQ, no RSS nor DCB */
> > > +	ETH_MQ_RX_VMDQ_ONLY = ETH_MQ_RX_VMDQ_FLAG,
> > > +	/**< RSS mode with VMDQ */
> > > +	ETH_MQ_RX_VMDQ_RSS = ETH_MQ_RX_RSS_FLAG |
> > ETH_MQ_RX_VMDQ_FLAG,
> > > +	/**< Use VMDQ+DCB to route traffic to queues */
> > > +	ETH_MQ_RX_VMDQ_DCB = ETH_MQ_RX_VMDQ_FLAG |
> > ETH_MQ_RX_DCB_FLAG,
> > > +	/**< Enable both VMDQ and DCB in VMDq */
> > > +	ETH_MQ_RX_VMDQ_DCB_RSS = ETH_MQ_RX_RSS_FLAG |
> > ETH_MQ_RX_DCB_FLAG |
> > > +				 ETH_MQ_RX_VMDQ_FLAG,
> > >  };
> > 
> > Why not simply remove all these combinations and keep only flags?
> > Please keep it simple.
> 
> One reason is back-compatibility.

I understand but I think we should prefer cleanup.
As there is no way to advertise deprecation of flags, it should be
simply removed.

> Another reason is not all NIC driver support all the combined modes, only limited sets
> driver supported. Under this condition, it's better to use the combination definition 
> (VMDQ_DCB, DCB_RSS, etc) to let driver check whether it supports.

Driver can do the same checks with simple flags and it's probably simpler
(e.g. a driver which doesn't support VMDQ had no need to check all VMDQ
combinations).

> > There are only few typos and minor things but it would help to have more
> > careful reviews. Having a list of people at the beginning of the patch
> > didn't help in this case.
> 
> I listed all the code reviewers out to reduce their workload to reply the email,
> not mean to make it easier to be applied.

I have no problem with listing of reviewers when submitting patches.
To say more, I prefer you list them by yourself and you add new reviewers
when sending new versions of the patchset.
But I would like reviewers to be more careful. They are especially useful to
discuss design choices and check typos.
Having reviewer give credits to the patch only if we are confident that the
review task is generally seriously achieved.

-- 
Thomas

  reply	other threads:[~2014-10-15  8:03 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23 13:14 [dpdk-dev] [PATCH 0/6] i40e " Chen Jing D(Mark)
2014-09-23 13:14 ` [dpdk-dev] [PATCH 1/6] ether: enhancement for " Chen Jing D(Mark)
2014-10-14 14:09   ` Thomas Monjalon
2014-10-15  6:59     ` Chen, Jing D
2014-10-15  8:10       ` Thomas Monjalon [this message]
2014-10-15  9:47         ` Chen, Jing D
2014-10-15  9:59           ` Thomas Monjalon
2014-10-16 10:07   ` [dpdk-dev] [PATCH v2 0/6] i40e " Chen Jing D(Mark)
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 1/6] ether: enhancement for " Chen Jing D(Mark)
2014-11-03 22:17       ` Thomas Monjalon
2014-11-04  5:50         ` Chen, Jing D
2014-11-04  8:53           ` Thomas Monjalon
2014-11-04  8:59             ` Chen, Jing D
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 2/6] igb: change for VMDQ arguments expansion Chen Jing D(Mark)
2014-11-03 18:37       ` Thomas Monjalon
2014-11-04  5:26         ` Chen, Jing D
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 3/6] ixgbe: " Chen Jing D(Mark)
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 4/6] i40e: add VMDQ support Chen Jing D(Mark)
2014-11-03 18:33       ` Thomas Monjalon
2014-11-04  5:22         ` Chen, Jing D
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 5/6] i40e: macaddr add/del enhancement Chen Jing D(Mark)
2014-10-16 10:07     ` [dpdk-dev] [PATCH v2 6/6] i40e: Add full VMDQ pools support Chen Jing D(Mark)
2014-10-21  3:30     ` [dpdk-dev] [PATCH v2 0/6] i40e VMDQ support Cao, Min
2014-11-03  7:54     ` Chen, Jing D
2014-11-04 10:01   ` [dpdk-dev] [PATCH v3 " Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 1/6] ether: enhancement for " Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 2/6] igb: change for VMDQ arguments expansion Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 3/6] ixgbe: " Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 4/6] i40e: add VMDQ support Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 5/6] i40e: macaddr add/del enhancement Chen Jing D(Mark)
2014-11-04 10:01     ` [dpdk-dev] [PATCH v3 6/6] i40e: Add full VMDQ pools support Chen Jing D(Mark)
2014-11-04 11:19     ` [dpdk-dev] [PATCH v3 0/6] i40e VMDQ support Ananyev, Konstantin
2014-11-04 23:17       ` Thomas Monjalon
2014-12-11  6:09     ` Cao, Min
2014-09-23 13:14 ` [dpdk-dev] [PATCH 2/6] igb: change for VMDQ arguments expansion Chen Jing D(Mark)
2014-09-23 13:14 ` [dpdk-dev] [PATCH 3/6] ixgbe: " Chen Jing D(Mark)
2014-09-23 13:14 ` [dpdk-dev] [PATCH 4/6] i40e: add VMDQ support Chen Jing D(Mark)
2014-10-13 16:14   ` De Lara Guarch, Pablo
2014-09-23 13:14 ` [dpdk-dev] [PATCH 5/6] i40e: macaddr add/del enhancement Chen Jing D(Mark)
2014-10-14 14:25   ` Thomas Monjalon
2014-10-15  7:01     ` Chen, Jing D
2014-09-23 13:14 ` [dpdk-dev] [PATCH 6/6] i40e: Add full VMDQ pools support Chen Jing D(Mark)
2014-10-10 10:45 ` [dpdk-dev] [PATCH 0/6] i40e VMDQ support Ananyev, Konstantin
2014-10-14  8:27 ` Chen, Jing D
2014-10-21  3:30 ` Cao, Min

Reply instructions:

You may reply publicly 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=10317086.OmtuOpvoDk@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=jing.d.chen@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

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://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/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


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