From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qc0-f182.google.com (mail-qc0-f182.google.com [209.85.216.182]) by dpdk.org (Postfix) with ESMTP id 221472A7 for ; Sat, 14 Jun 2014 11:00:10 +0200 (CEST) Received: by mail-qc0-f182.google.com with SMTP id m20so5287527qcx.41 for ; Sat, 14 Jun 2014 02:00:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=+dqwf2mEEYdAKvNMNqMgsNWLbmX2pZCbjphE8qQLZu8=; b=qCvavsnOwAR/pgnEwZQR8FYzQhO6pi8IJ7GFIPltOlhssIVqeMtrXaI2+X76IRyVAf HpK0us5QahZqZL0QBrzHuxfBhPJKFoeIoWkvD2oYYO93Kj72jccENZPGIBsU7Gp4aOaZ bMKQDfnCba7v2f1eC3Vn1ry5H+Kn7tTY+dwrHOx1AO+CVT5Q5+CwTA9w79vksbUATpr4 tADjzkQOYoBx757gakze6o38pl3WPRScW92lRLTbN9v36p7bCdAjGXvBuSnXNRccLB/A /EdpEtR82A9/DWwPn64tzgBIOblJeue2ogA/6WX+o0wNvdqG0QnR+8hSiJIA/eVPpn2A gDhA== MIME-Version: 1.0 X-Received: by 10.140.88.18 with SMTP id s18mr34018qgd.73.1402736425491; Sat, 14 Jun 2014 02:00:25 -0700 (PDT) Received: by 10.140.30.55 with HTTP; Sat, 14 Jun 2014 02:00:25 -0700 (PDT) In-Reply-To: <9BB6961774997848B5B42BEC655768F8A8D780@SHSMSX104.ccr.corp.intel.com> References: <1400895442-32433-1-git-send-email-jingjing.wu@intel.com> <2116871.SYCYqgDKRI@xps13> <9BB6961774997848B5B42BEC655768F8A8BBB5@SHSMSX104.ccr.corp.intel.com> <27520622.tiYQNKLyvh@xps13> <9BB6961774997848B5B42BEC655768F8A8D402@SHSMSX104.ccr.corp.intel.com> <9BB6961774997848B5B42BEC655768F8A8D780@SHSMSX104.ccr.corp.intel.com> Date: Sat, 14 Jun 2014 13:00:25 +0400 Message-ID: From: Vladimir Medvedkin To: "Wu, Jingjing" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Jun 2014 09:00:10 -0000 Hi Jingjing, Ok! Let's get back to this patch after 1.7 release. Thanks! Regards, Vladimir 2014-06-14 5:00 GMT+04:00 Wu, Jingjing : > Hi, Vladimir > > > > Yes, for Fortville, uint8_t is not enough, it was also the concern is to > keep consistent with flow director=E2=80=99s implementation. But I agree = that we > need to change. > > > > Let make an agreement like: > > > > I will make change for the remarks from you. One is the change the type o= f > rx_queue to uint16_t. The other is change API like > =E2=80=9Crte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filte= r *filter, > uint16_t rx_queue)=E2=80=9D. > > > > And about the pool and virtualization case, maybe you will send a new > patch about it, maybe me. Whatever, just leave it in future, not include > in this patch. > > > > Thank you! > > Jingjing > > > > *From:* Vladimir Medvedkin [mailto:medvedkinv@gmail.com] > *Sent:* Saturday, June 14, 2014 12:19 AM > *To:* Wu, Jingjing > *Cc:* Thomas Monjalon; dev@dpdk.org > > *Subject:* Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic > filter > > > > Hi Jingjing, > > Yes, I agree. > I have one more remark. It is about type of rx_queue arg. Now it is > uint8_t. I think we have to change it to uint16_t because for Fortville N= IC > it is not enough. Quote fro the datasheet: > A PF VSI (Virtual Station Interfaces aka virtual NICs) can allocate and > use up to 1536 LQPs (LAN queue pairs). > > Regards, > > Vladimir > > > > 2014-06-13 18:12 GMT+04:00 Wu, Jingjing : > > Hi, Vladimir > > > > Thanks a lot for your remarks. > > > > Yes, your understanding is correct, in non-IOV mode, we can use 64pool, > per pool can has 2 queues when ETH_MQ_RX_VMDQ_ONLY. While in IOV mode, > current DPDK version makes the number of queue to 1 by default. The pools > logic makes sense, but I didn=E2=80=99t consider it globally with the thi= nking we > can do it in future. I will be great if you can generate a new patch base= d > on mine. Or we can discuss it further? Due to it is close to the feature > deliver time now and much verification work for it, it may not possible t= o > add it in this patch. > > > > In API > > About your first remark, the reason why I didn=E2=80=99t put the queue in= the > filter structure is that the filter contains the fields used for comparis= on > and the queue is acted as result, and another concern is to keep consiste= nt > with flow director=E2=80=99s implementation. > > About your second remark, I will accept it and integrate the change to > patch in new version. > > > > Do your agree my proposal? > > > > > > *From:* Vladimir Medvedkin [mailto:medvedkinv@gmail.com] > *Sent:* Friday, June 13, 2014 7:52 PM > *To:* Thomas Monjalon > *Cc:* Wu, Jingjing; dev@dpdk.org > > > *Subject:* Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic > filter > > > > Hi all, > > The 82599 datasheet (p. 284 and p.287) has only recommendations and only > when possible about assign rx queue not used by RSS/DCB. I do not see any > serious restrictions do not assign the rx queue used by RSS/DCB. > > For cases with only 1 queue if I understand correctly this patch > http://dpdk.org/ml/archives/dev/2014-May/002589.html we can init second > queue in pool and assign it by filter. In *ETH_MQ_RX_VMDQ_ONLY* mode > init all possible queues (even if hardware route packets to zero queue in > pools) so there no problem. Moreover, it is not necesary for rx queue to = be > set in the same pool. > > > About genericity. I agree with Jingjing, different controllers have > different definitions for pools or VFs. And it is only Intel controllers! > It is very hard to predict hardware implementation. For example for > Fortville I can not find 5-tuple filters at all. > > > > API. I have several remarks. > > 1. You use rx_queue as separate arg. For example: > > rte_eth_dev_add_ethertype_filter(uint8_t port_id, uint16_t index, struct > rte_ethertype_filter *filter, uint8_t rx_queue) > rte_eth_dev_get_ethertype_filter(uint8_t port_id, uint16_t index, struct > rte_ethertype_filter *filter, uint8_t *rx_queue) > > you can move uint8_t rx_queue into struct rte_ethertype_filter *filter. > > 2. In SYN filter: > rte_eth_dev_add_syn_filter(uint8_t port_id, uint8_t high_pri, uint8_t > rx_queue) > rte_eth_dev_get_syn_filter(uint8_t port_id, struct rte_syn_filter *filter= , > uint8_t *rx_queue) > > In first ADD func you alloc struct rte_syn_filter inside func, but in GET > func you have to alloc struct rte_syn_filter in your app. May be better t= o > do > rte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filter *filter= , > uint8_t *rx_queue) ? > > > > So, Jingjing made a lot of work, much more then I (igb filters, testpmd > commands). It works the same as mine (not counting pools logic), so let's > integrate it (it's will be great if jingjing change api according to my > remarks). > > > > Regards, > > Vladimir > > > > 2014-06-12 19:36 GMT+04:00 Thomas Monjalon : > > > 2014-06-11 17:45, Thomas Monjalon: > > > > My main concern is that Vladimir Medvedkin suggested another API and > I'd > > > like you give your opinion about it: > > > http://dpdk.org/ml/archives/dev/2014-June/003053.html > > > It offers pool number in configuration of the filters. > > 2014-06-12 08:08, Wu, Jingjing: > > > The pool field is used in virtualization scenario. It is acting as one = of > > input set during filter matching in ixgbe. > > My patch didn't consider the virtualization scenario in generic filter > > feature. Because in 82599 datasheet, it is recommended to assign rx > queues > > not used by DCB/RSS, that is virtualization without RSS and DCB mode. F= or > > this mode, current DPDK version makes the number of queue to 1 by > default in > > IOV mode. So in this case it makes no sense make pool as a input set an= d > the > > rx queue also need to be set to in this pool, so just keep the consiste= nt > > with flow director who also ignore it in previous version. > > And further E1000/Niantic/Fortville have different definitions for VF, = we > > need to think how to define it more generic. > > And even just need offer pool number in configuration of the filters as > what > > Vladimir did, it also need to verify the interworking with Virtualizati= on > > for different kinds of NICs, and the interworking with DCB and RSS whic= h > is > > not recommended in 82599's datasheet. > > So I think it will be a good choice to implement generic filter > interworking > > with virtualization in future patch. If there is any volunteer to send > patch > > for support this concern later, it will be also cool. > > Vladimir, do you agree with this analysis? > As you suggested another implementation, I need you acknowledgment for th= is > patchset to be integrated. > > Thanks > -- > Thomas > > > > >