DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wu, Jingjing" <jingjing.wu@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure
Date: Thu, 13 Nov 2014 05:44:13 +0000	[thread overview]
Message-ID: <9BB6961774997848B5B42BEC655768F8B47F69@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <4699150.INYaCBBBvY@xps13>

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

  reply	other threads:[~2014-11-13  5:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25  6:59 [dpdk-dev] [PATCH 0/4] support control packet filter on Fortville Jingjing Wu
2014-09-25  6:59 ` [dpdk-dev] [PATCH 1/4] lib/librte_ether: new filter APIs definition Jingjing Wu
2014-10-09 15:34   ` De Lara Guarch, Pablo
2014-10-10  1:19     ` Wu, Jingjing
2014-10-10  3:34     ` Wu, Jingjing
2014-10-10  7:28       ` De Lara Guarch, Pablo
2014-10-16 15:11         ` Thomas Monjalon
2014-09-25  6:59 ` [dpdk-dev] [PATCH 2/4] lib/librte_ether: define CTRL_PKT filter type and its structure Jingjing Wu
2014-09-25  6:59 ` [dpdk-dev] [PATCH 3/4] i40e: ctrl_pkt filter implementation in i40e pmd driver Jingjing Wu
2014-09-25  6:59 ` [dpdk-dev] [PATCH 4/4] app/test-pmd: add commands for control packet filter Jingjing Wu
2014-09-25  7:54 ` [dpdk-dev] [PATCH 0/4] support control packet filter on Fortville Zhang, Helin
2014-10-11  7:23 ` Chen, Jing D
2014-10-11  7:23 ` Liu, Jijiang
2014-10-11  7:53 ` Zhang, Helin
2014-10-22  8:19 ` [dpdk-dev] [PATCH v2 0/3] support control packet filter on fortville Jingjing Wu
2014-10-22  8:19   ` [dpdk-dev] [PATCH v2 1/3] ethdev: define ctrl_pkt filter type and its structure Jingjing Wu
2014-10-30 22:47     ` Thomas Monjalon
2014-10-31  7:05       ` Wu, Jingjing
2014-10-31  8:44         ` Thomas Monjalon
2014-11-13  5:44           ` Wu, Jingjing [this message]
2014-11-13  8:41             ` Thomas Monjalon
2014-11-13  9:00               ` Wu, Jingjing
2014-10-22  8:19   ` [dpdk-dev] [PATCH v2 2/3] i40e: ctrl_pkt filter implementation in i40e pmd driver Jingjing Wu
2014-10-22  8:19   ` [dpdk-dev] [PATCH v2 3/3] testpmd: Commands to test ctrl_pkt filter Jingjing Wu
2014-11-13 12:49   ` [dpdk-dev] [PATCH v3 0/2] support ethertype filter on fortville Jingjing Wu
2014-11-13 12:49     ` [dpdk-dev] [PATCH v3 1/2] ethdev: new structure of Ethertype Filter for filter_ctrl api Jingjing Wu
2014-11-27  5:21       ` Qiu, Michael
2014-11-13 12:49     ` [dpdk-dev] [PATCH v3 2/2] i40e: implement operation to add/delete an ethertype filter Jingjing Wu
2014-11-18 13:46     ` [dpdk-dev] [PATCH v3 0/2] support ethertype filter on fortville Liu, Jijiang
2014-11-26 22:24       ` Thomas Monjalon

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=9BB6961774997848B5B42BEC655768F8B47F69@SHSMSX104.ccr.corp.intel.com \
    --to=jingjing.wu@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).