From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 2162CD4E0 for ; Wed, 11 Jan 2017 09:55:15 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP; 11 Jan 2017 00:55:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,345,1477983600"; d="scan'208";a="1110966633" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by fmsmga002.fm.intel.com with ESMTP; 11 Jan 2017 00:55:13 -0800 Received: from pgsmsx103.gar.corp.intel.com ([169.254.2.52]) by PGSMSX108.gar.corp.intel.com ([169.254.8.13]) with mapi id 14.03.0248.002; Wed, 11 Jan 2017 16:54:46 +0800 From: "Zhao1, Wei" To: "Yigit, Ferruh" , "dev@dpdk.org" CC: "Lu, Wenzhuo" Thread-Topic: [dpdk-dev] [PATCH v2 12/18] net/ixgbe: parse ethertype filter Thread-Index: AQHSYnKDdPnEhTFWjUSgqjBTXA+UYaErNMmAgAfV1EA= Date: Wed, 11 Jan 2017 08:54:46 +0000 Message-ID: References: <1483084390-53159-1-git-send-email-wei.zhao1@intel.com> <1483084390-53159-13-git-send-email-wei.zhao1@intel.com> <155c938b-0d58-0eba-bc0f-7851429f0404@intel.com> In-Reply-To: <155c938b-0d58-0eba-bc0f-7851429f0404@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.30.20.205] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 12/18] net/ixgbe: parse ethertype filter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Jan 2017 08:55:16 -0000 Hi, Yigit > -----Original Message----- > From: Yigit, Ferruh > Sent: Saturday, January 7, 2017 1:11 AM > To: Zhao1, Wei ; dev@dpdk.org > Cc: Lu, Wenzhuo > Subject: Re: [dpdk-dev] [PATCH v2 12/18] net/ixgbe: parse ethertype filte= r >=20 > On 12/30/2016 7:53 AM, Wei Zhao wrote: > > check if the rule is a ethertype rule, and get the ethertype info. > > Signed-off-by: Wei Zhao > > Signed-off-by: Wenzhuo Lu > > > > --- > > > > v2:add new error set function > > --- >=20 > <...> >=20 > > +static int > > +ixgbe_parse_ethertype_filter(const struct rte_flow_attr *attr, > > + const struct rte_flow_item pattern[], > > + const struct rte_flow_action actions[], > > + struct rte_eth_ethertype_filter *filter, > > + struct rte_flow_error *error) { > > + int ret; > > + > > + ret =3D cons_parse_ethertype_filter(attr, pattern, > > + actions, filter, error); > > + > > + if (ret) > > + return ret; > > + > > + /* Ixgbe doesn't support MAC address. */ > > + if (filter->flags & RTE_ETHTYPE_FLAGS_MAC) { > > + memset(filter, 0, sizeof(struct rte_eth_ethertype_filter)); >=20 > Is memset required for error cases, if so is other error cases also requi= re it? Yes, Ok , I will do as your suggestion. >=20 > > + rte_flow_error_set(error, EINVAL, > > + RTE_FLOW_ERROR_TYPE_ITEM, > > + NULL, "Not supported by ethertype filter"); > > + return -rte_errno; > > + } > > + > > + if (filter->queue >=3D IXGBE_MAX_RX_QUEUE_NUM) > > + return -rte_errno; > > + > > + if (filter->ether_type =3D=3D ETHER_TYPE_IPv4 || > > + filter->ether_type =3D=3D ETHER_TYPE_IPv6) { > > + PMD_DRV_LOG(ERR, "unsupported ether_type(0x%04x) in" > > + " ethertype filter.", filter->ether_type); >=20 > Not sure about this log here, specially it is in ERR level. > This function is returning error, and public API will return an error, if= we want > to notify user with a log, should app do this as library > (here) should do this? More comment welcome? >=20 > btw, should rte_flow_error_set() used here (and below) ? Yes, I will change to rte_flow_error_set()=20 >=20 > > + return -rte_errno; > > + } > > + > > + if (filter->flags & RTE_ETHTYPE_FLAGS_MAC) { >=20 > Isn't this duplicate with above check? >=20 > > + PMD_DRV_LOG(ERR, "mac compare is unsupported."); > > + return -rte_errno; > > + } > > + > > + if (filter->flags & RTE_ETHTYPE_FLAGS_DROP) { >=20 > Just to double check, isn't drop action by ether filters? Yse , ixgbe do not, but i40e is. >=20 > > + PMD_DRV_LOG(ERR, "drop option is unsupported."); > > + return -rte_errno; > > + } > > + > > + return 0; > > +} > > + > > +/** > <...>