2014-10-15 06:59, Chen, Jing D: > From: Thomas Monjalon [mailto:thomas.monjalon at 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
