Message ID | 1413965977-15165-2-git-send-email-jingjing.wu@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id C22117E80; Wed, 22 Oct 2014 10:12:35 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 99D047E7B for <dev@dpdk.org>; Wed, 22 Oct 2014 10:12:33 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 22 Oct 2014 01:19:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,767,1406617200"; d="scan'208";a="623038007" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by orsmga002.jf.intel.com with ESMTP; 22 Oct 2014 01:20:17 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id s9M8KFft004968; Wed, 22 Oct 2014 16:20:15 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id s9M8KClm015209; Wed, 22 Oct 2014 16:20:15 +0800 Received: (from wujingji@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id s9M8KCuK015205; Wed, 22 Oct 2014 16:20:12 +0800 From: Jingjing Wu <jingjing.wu@intel.com> To: dev@dpdk.org Date: Wed, 22 Oct 2014 16:19:35 +0800 Message-Id: <1413965977-15165-2-git-send-email-jingjing.wu@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1413965977-15165-1-git-send-email-jingjing.wu@intel.com> References: <1411628369-29532-1-git-send-email-jingjing.wu@intel.com> <1413965977-15165-1-git-send-email-jingjing.wu@intel.com> Subject: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Jingjing Wu
Oct. 22, 2014, 8:19 a.m. UTC
define new filter type and its structure
- RTE_ETH_FILTER_CTRL_PKT
- struct rte_ctrl_pkt_filter
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
lib/librte_ether/rte_eth_ctrl.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
Comments
2014-10-22 16:19, Jingjing Wu: > +/** > + * Define all structures for Control Packet Filter type corresponding with specific operations. > + */ Please explain what is a control packet. > + > +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 > +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 > +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 > +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 > +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 Flag RX is 0? > +/** > + * A structure used to define the control packet filter entry > + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD > + * and RTE_ETH_FILTER_DELETE operations. > + */ > +struct rte_ctrl_pkt_filter { > + struct ether_addr mac_addr; /**< mac address to match. */ > + uint16_t ether_type; /**< ether type to match */ > + uint16_t flags; /**< options for filter's behavior*/ > + uint16_t dest_id; /**< destination vsi id or pool id*/ vsi id and pool id cannot be understood in a generic context. Please explain what you mean and why queue is not enough. > + uint16_t queue; /**< queue assign to if TO QUEUE flag is set */ TO QUEUE is not defined. Is it really needed? Thanks
Hi, Thomas Thanks for your comments Jingjing > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, October 31, 2014 6:47 AM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type > and its structure > > 2014-10-22 16:19, Jingjing Wu: > > +/** > > + * Define all structures for Control Packet Filter type corresponding with > specific operations. > > + */ > > Please explain what is a control packet. A control element in Fortville can be used to receive control packets and control other switching elements. Control packet filter can filter control packet (such as LLDP) to different queues in receive and identify the switch element that should process the packets in transmit. At the same time, we also can use this filter to route non-control packets to queue or other engine by filtering mac and ether_type. The name "control packet filter" comes from Fortville. > > > + > > +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 > > +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 > > +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 > > +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 > > +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 > > Flag RX is 0? Yes, RX is default value. Maybe it need to be removed. > > > +/** > > + * A structure used to define the control packet filter entry > > + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD > > + * and RTE_ETH_FILTER_DELETE operations. > > + */ > > +struct rte_ctrl_pkt_filter { > > + struct ether_addr mac_addr; /**< mac address to match. */ > > + uint16_t ether_type; /**< ether type to match */ > > + uint16_t flags; /**< options for filter's behavior*/ > > + uint16_t dest_id; /**< destination vsi id or pool id*/ > > vsi id and pool id cannot be understood in a generic context. > Please explain what you mean and why queue is not enough. If queue is not specified, dest_id defines which element (vsi) will get the packet. If queue is specified, the queue need belong to the destination element. > > > + uint16_t queue; /**< queue assign to if TO QUEUE flag is set > */ > > TO QUEUE is not defined. Is it really needed? > TO QUEUE is just the flag RTE_CONTROL_PACKET_FLAGS_TO_QUEUE above. > Thanks > -- > Thomas
Hi Jingjing, I'm sorry, but your explanations are not sufficient. Please keep in mind that the user of the API don't know i40e internals. 2014-10-31 07:05, Wu, Jingjing: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2014-10-22 16:19, Jingjing Wu: > > > +/** > > > + * Define all structures for Control Packet Filter type corresponding with > > specific operations. > > > + */ > > > > Please explain what is a control packet. > > A control element in Fortville can be used to receive control packets and control other switching elements. Control packet filter can filter control packet (such as LLDP) to different queues in receive and identify the switch element that should process the packets in transmit. > At the same time, we also can use this filter to route non-control packets to queue or other engine by filtering mac and ether_type. The name "control packet filter" comes from Fortville. I still don't know what is a control packet. > > > +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 > > > +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 > > > +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 > > > +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 > > > +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 > > > > Flag RX is 0? > > Yes, RX is default value. Maybe it need to be removed. No, it doesn't need to be removed. But a flag must not be 0. 0 means none. It's impossible to disable this flag. Moreover, you should comment each flag. > > > +/** > > > + * A structure used to define the control packet filter entry > > > + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD > > > + * and RTE_ETH_FILTER_DELETE operations. > > > + */ > > > +struct rte_ctrl_pkt_filter { > > > + struct ether_addr mac_addr; /**< mac address to match. */ > > > + uint16_t ether_type; /**< ether type to match */ > > > + uint16_t flags; /**< options for filter's behavior*/ > > > + uint16_t dest_id; /**< destination vsi id or pool id*/ > > > > vsi id and pool id cannot be understood in a generic context. > > Please explain what you mean and why queue is not enough. > > If queue is not specified, dest_id defines which element (vsi) will get the packet. > If queue is specified, the queue need belong to the destination element. You really need to define what is a vsi id and pool id. These notions are not known in the API layer. > > > + uint16_t queue; /**< queue assign to if TO QUEUE flag is set > > */ > > > > TO QUEUE is not defined. Is it really needed? > > > TO QUEUE is just the flag RTE_CONTROL_PACKET_FLAGS_TO_QUEUE above. OK, please use the same wording or users will get lost.
Hi, Thomas The input set of control packet filter are dst_mac and ethertype in Ethernet head. To be clear, I think it's better to use the name ethertype filter. While there is already ethertype filter existing in igb and ixgbe driver. I will rename The patchset to ethertype filter and also integrate igb and ixgbe's ethertype filter To the filter_ctrl API. What do you think? Jingjing > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, October 31, 2014 4:45 PM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type > and its structure > > Hi Jingjing, > > I'm sorry, but your explanations are not sufficient. > Please keep in mind that the user of the API don't know i40e internals. > > 2014-10-31 07:05, Wu, Jingjing: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > 2014-10-22 16:19, Jingjing Wu: > > > > +/** > > > > + * Define all structures for Control Packet Filter type > > > > +corresponding with > > > specific operations. > > > > + */ > > > > > > Please explain what is a control packet. > > > > A control element in Fortville can be used to receive control packets and > control other switching elements. Control packet filter can filter control > packet (such as LLDP) to different queues in receive and identify the switch > element that should process the packets in transmit. > > At the same time, we also can use this filter to route non-control packets to > queue or other engine by filtering mac and ether_type. The name "control > packet filter" comes from Fortville. > > I still don't know what is a control packet. > > > > > +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 > > > > +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 > > > > +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 > > > > +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 > > > > +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 > > > > > > Flag RX is 0? > > > > Yes, RX is default value. Maybe it need to be removed. > > No, it doesn't need to be removed. But a flag must not be 0. > 0 means none. > It's impossible to disable this flag. > > Moreover, you should comment each flag. > > > > > +/** > > > > + * A structure used to define the control packet filter entry > > > > + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD > > > > + * and RTE_ETH_FILTER_DELETE operations. > > > > + */ > > > > +struct rte_ctrl_pkt_filter { > > > > + struct ether_addr mac_addr; /**< mac address to match. */ > > > > + uint16_t ether_type; /**< ether type to match */ > > > > + uint16_t flags; /**< options for filter's behavior*/ > > > > + uint16_t dest_id; /**< destination vsi id or pool id*/ > > > > > > vsi id and pool id cannot be understood in a generic context. > > > Please explain what you mean and why queue is not enough. > > > > If queue is not specified, dest_id defines which element (vsi) will get the > packet. > > If queue is specified, the queue need belong to the destination element. > > You really need to define what is a vsi id and pool id. These notions are not > known in the API layer. > > > > > + uint16_t queue; /**< queue assign to if TO QUEUE flag is set > > > */ > > > > > > TO QUEUE is not defined. Is it really needed? > > > > > TO QUEUE is just the flag RTE_CONTROL_PACKET_FLAGS_TO_QUEUE > above. > > OK, please use the same wording or users will get lost. > > -- > Thomas
Hi Jingjing, 2014-11-13 05:44, Wu, Jingjing: > The input set of control packet filter are dst_mac and ethertype in Ethernet head. > To be clear, I think it's better to use the name ethertype filter. > > While there is already ethertype filter existing in igb and ixgbe driver. I will rename > The patchset to ethertype filter and also integrate igb and ixgbe's ethertype filter > To the filter_ctrl API. > > What do you think? OK, good. If I understand well, this feature is now planned for release 2.0?
Hi, Thomas Nop. We target to have it in r1.8. And the whole patch set is almost ready, but lack of review internally. I have two proposes here: 1. Send a patch set include all the ethdev, i40e, ixgbe, igb and testpmd changes just as my previous mail. 2. send a patch set only include the ethdev, i40e part. Without testpmd changes to support testing it in fortville. And will send remaining changes later. Maybe r2.0? The second proposal will split the whole task to small patchset. And easy to review. Which one do you prefer? Look forward to your reply! Thanks Jingjing > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, November 13, 2014 4:41 PM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure > > Hi Jingjing, > > 2014-11-13 05:44, Wu, Jingjing: > > The input set of control packet filter are dst_mac and ethertype in Ethernet head. > > To be clear, I think it's better to use the name ethertype filter. > > > > While there is already ethertype filter existing in igb and ixgbe driver. I will rename > > The patchset to ethertype filter and also integrate igb and ixgbe's ethertype filter > > To the filter_ctrl API. > > > > What do you think? > > OK, good. > If I understand well, this feature is now planned for release 2.0? > > -- > Thomas
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h index df21ac6..8d3dba9 100644 --- a/lib/librte_ether/rte_eth_ctrl.h +++ b/lib/librte_ether/rte_eth_ctrl.h @@ -51,6 +51,7 @@ extern "C" { */ enum rte_filter_type { RTE_ETH_FILTER_NONE = 0, + RTE_ETH_FILTER_CTRL_PKT, RTE_ETH_FILTER_MAX }; @@ -71,6 +72,29 @@ enum rte_filter_op { RTE_ETH_FILTER_OP_MAX }; +/** + * Define all structures for Control Packet Filter type corresponding with specific operations. + */ + +#define RTE_CONTROL_PACKET_FLAGS_IGNORE_MAC 0x0001 +#define RTE_CONTROL_PACKET_FLAGS_DROP 0x0002 +#define RTE_CONTROL_PACKET_FLAGS_TO_QUEUE 0x0004 +#define RTE_CONTROL_PACKET_FLAGS_TX 0x0008 +#define RTE_CONTROL_PACKET_FLAGS_RX 0x0000 + +/** + * A structure used to define the control packet filter entry + * to support RTE_ETH_FILTER_CTRL_PKT with RTE_ETH_FILTER_ADD + * and RTE_ETH_FILTER_DELETE operations. + */ +struct rte_ctrl_pkt_filter { + struct ether_addr mac_addr; /**< mac address to match. */ + uint16_t ether_type; /**< ether type to match */ + uint16_t flags; /**< options for filter's behavior*/ + uint16_t dest_id; /**< destination vsi id or pool id*/ + uint16_t queue; /**< queue assign to if TO QUEUE flag is set */ +}; + #ifdef __cplusplus } #endif