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.
> > + 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
