Re: [ovs-dev] [PATCH v4 4/9] dp-packet: Add flow_mark support for non-DPDK case.

[Ilya Maximets]

On 17.02.2019 16:44, Asaf Penso wrote:
> 
> 
> Regards,
> Asaf Penso
> 
>> -----Original Message-----
>> From: ovs-dev-boun...@openvswitch.org <ovs-dev-
>> boun...@openvswitch.org> On Behalf Of Flavio Leitner
>> Sent: Friday, February 15, 2019 8:59 PM
>> To: Ilya Maximets <i.maxim...@samsung.com>
>> Cc: ovs-dev@openvswitch.org; Roni Bar Yanai <ron...@mellanox.com>
>> Subject: Re: [ovs-dev] [PATCH v4 4/9] dp-packet: Add flow_mark support for
>> non-DPDK case.
>>
>> On Fri, Feb 15, 2019 at 04:07:03PM +0300, Ilya Maximets wrote:
>>> Additionally, new API call 'dp_packet_set_flow_mark' is needed for
>>> packet clone. Mostly for dummy HWOL implementation.
>>>
>>> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
>>> ---
>>>  lib/dp-packet.h | 23 +++++++++++++++++++++--
>>>  1 file changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
>>> 685edc90c..15c596a91 100644
>>> --- a/lib/dp-packet.h
>>> +++ b/lib/dp-packet.h
>>> @@ -60,7 +60,9 @@ struct dp_packet {
>>>      uint32_t size_;             /* Number of bytes in use. */
>>>      uint32_t ol_flags;          /* Offloading flags. */
>>>  #define DP_PACKET_OL_RSS_HASH_MASK   0x1 /* Is the 'rss_hash' valid?
>> */
>>> +#define DP_PACKET_OL_FLOW_MARK_MASK  0x2 /* Is the 'flow_mark'
>> valid?
>>> +*/
>>
>> Perhaps it's time for a enum?
>>
>> Otherwise it looks good to me.
>> fbl
>>
> 
> I also think that enum is a good approach.

OK. I'll turn this to enum.

> 
>>
>>>      uint32_t rss_hash;          /* Packet hash. */
>>> +    uint32_t flow_mark;         /* Packet flow mark. */
>>>  #endif
>>>      enum dp_packet_source source;  /* Source of memory allocated as
>>> 'base'. */
>>>
>>> @@ -556,6 +558,13 @@ dp_packet_has_flow_mark(const struct
>> dp_packet *p, uint32_t *mark)
>>>      return false;
>>>  }
>>>
>>> +static inline void
>>> +dp_packet_set_flow_mark(struct dp_packet *p, uint32_t mark) {
>>> +    p->mbuf.hash.fdir.hi = mark;
>>> +    p->mbuf.ol_flags |= PKT_RX_FDIR_ID; }
>>> +
>>>  #else /* DPDK_NETDEV */
>>>  static inline void *
>>>  dp_packet_base(const struct dp_packet *b) @@ -657,11 +666,21 @@
>>> dp_packet_l4_checksum_bad(const struct dp_packet *p OVS_UNUSED)  }
>>>
>>>  static inline bool
>>> -dp_packet_has_flow_mark(const struct dp_packet *p OVS_UNUSED,
>>> -                        uint32_t *mark OVS_UNUSED)
>>> +dp_packet_has_flow_mark(const struct dp_packet *p, uint32_t *mark)
>>>  {
>>> +    if (p->ol_flags & DP_PACKET_OL_FLOW_MARK_MASK) {
>>> +        *mark = p->flow_mark;
>>> +        return true;
>>> +    }
>>>      return false;
>>>  }
>>> +
>>> +static inline void
>>> +dp_packet_set_flow_mark(struct dp_packet *p, uint32_t mark) {
>>> +    p->flow_mark = mark;
>>> +    p->ol_flags |= DP_PACKET_OL_FLOW_MARK_MASK; }
>>>  #endif /* DPDK_NETDEV */
>>>
>>>  static inline void
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> dev mailing list
>>> d...@openvswitch.org
>>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
>> i
>>> l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
>> dev&amp;data=02%7C01%7Cas
>>>
>> afp%40mellanox.com%7C82821a54567740211aaf08d69377a00d%7Ca652971c7
>> d2e4d
>>>
>> 9ba6a4d149256f461b%7C0%7C0%7C636858539332346581&amp;sdata=mEyH3
>> EnR9dIh
>>> Rq5WxldGHJayVcnjp7RMdbqjiQXVaG8%3D&amp;reserved=0
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
>> il.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
>> dev&amp;data=02%7C01%7Casafp%40mellanox.com%7C82821a54567740211
>> aaf08d69377a00d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63
>> 6858539332356590&amp;sdata=HK%2FlvDuDociyZG7K79kbjOQ0TOKr%2Bmk
>> MIyHvlYZCj%2BI%3D&amp;reserved=0
> 
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev