On Mon, Sep 11, 2017 at 01:37:10PM +0800, Yuanhan Liu wrote: > On Fri, Sep 08, 2017 at 05:38:07PM +0000, Darrell Ball wrote: > > > +static inline bool > > > +dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED, > > > + uint32_t *mark OVS_UNUSED) > > > +{ > > > +#ifdef DPDK_NETDEV > > > + if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) { > > > + *mark = p->mbuf.hash.fdir.hi; > > > + return true; > > > + } > > > +#endif > > > + return false; > > > +} > > > > It seems odd that p and mark are marked as OVS_UNUSED > > but used in the case that DPDK_NETDEV is defined. > > > > [Darrell] OVS_UNUSED means ‘’may be unused” > > Typically, for a trivial alternative, we would do a slight > > variation of the above > > rather than writing two versions of the functions. > > Right. I also saw quite many examples like this in OVS. That's why I > code in this way, though I agree with Simon, it's indeed a bit odd.
Thanks. In light of the above I have no objections to dp_packet_has_flow_mark() the way it is. > > > + if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER))) { > > > + if (OVS_LIKELY(nw_proto == IPPROTO_TCP)) { > > > + if (OVS_LIKELY(size >= TCP_HEADER_LEN)) { > > > > The following might be nicer: > > > > + if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER)) > > + && OVS_LIKELY(nw_proto == IPPROTO_TCP) > > + && OVS_LIKELY(size >= TCP_HEADER_LEN)) { > > Indeed. Thanks! > > --yliu _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev