Hi Adrien,

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com]
> Sent: Wednesday, August 30, 2017 1:39 PM
> To: Iremonger, Bernard <bernard.iremon...@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yi...@intel.com>; Ananyev,
> Konstantin <konstantin.anan...@intel.com>; Dumitrescu, Cristian
> <cristian.dumitre...@intel.com>
> Subject: Re: [PATCH v2 3/6] librte_ether: initialise IPv4 protocol mask for
> rte_flow
> 
> Hi Bernard,
> 
> On Fri, Aug 25, 2017 at 05:10:35PM +0100, Bernard Iremonger wrote:
> > Initialise the next_proto_id mask in the default mask for
> > rte_flow_item_type_ipv4.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremon...@intel.com>
> > ---
> >  lib/librte_ether/rte_flow.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> > index bba6169..59c42fa 100644
> > --- a/lib/librte_ether/rte_flow.h
> > +++ b/lib/librte_ether/rte_flow.h
> > @@ -489,6 +489,7 @@ struct rte_flow_item_ipv4 {  #ifndef __cplusplus
> > static const struct rte_flow_item_ipv4 rte_flow_item_ipv4_mask = {
> >     .hdr = {
> > +           .next_proto_id = 0xff,
> 
> Please don't change the default mask to cover this field as it means
> all rte_flow-based applications that do not provide a specific mask
> (.mask == NULL) have to always set this field to some valid value.
> This is not a convenient default behavior.
> 
> >             .src_addr = RTE_BE32(0xffffffff),
> >             .dst_addr = RTE_BE32(0xffffffff),
> >     },
> > --
> > 1.9.1
> >
> 
> I'll have to NACK this change. The example application should define its own
> mask if next_proto_id must be always set.

Surely for IPv4 the next_proto_id will always be set to TCP(6) , UDP(17) or 
SCTP (132).
If the mask is 0 for next_proto_id then it is not possible to match on the 
protocol.
I can define an ipv4_mask in the application if you insist. 

Regards,

Bernard.

Reply via email to