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 at 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
