On 05.02.2019 16:09, Asaf Penso wrote:
>
>
> Regards,
> Asaf Penso
>
>> -----Original Message-----
>> From: Ilya Maximets <i.maxim...@samsung.com>
>> Sent: Tuesday, February 5, 2019 1:46 PM
>> To: Asaf Penso <as...@mellanox.com>; ovs-dev@openvswitch.org
>> Cc: Roni Bar Yanai <ron...@mellanox.com>; Stokes, Ian
>> <ian.sto...@intel.com>; Finn Christensen <f...@napatech.com>
>> Subject: Re: [ovs-dev,v2] netdev-dpdk: Memset rte_flow_item on a need
>> basis.
>>
>> On 04.02.2019 19:14, Asaf Penso wrote:
>>> In netdev_dpdk_add_rte_flow_offload function different rte_flow_item
>>> are created as part of the pattern matching.
>>>
>>> For most of them, there is a check whether the wildcards are not zero.
>>> In case of zero, nothing is being done with the rte_flow_item.
>>>
>>> Befor the wildcard check, and regardless of the result, the
>>> rte_flow_item is being memset.
>>>
>>> The patch moves the memset function only if the condition of the
>>> wildcard is true, thus saving cycles of memset if not needed.
>>>
>>> Signed-off-by: Asaf Penso <as...@mellanox.com>
>>> ---
>>
>> I thought about this part of code a bit. IMHO, we could create a union from
>> the L4 items and clear them all at once. Like this:
>>
>> uint8_t proto = 0;
>> struct flow_items {
>> struct rte_flow_item_eth eth;
>> struct rte_flow_item_vlan vlan;
>> struct rte_flow_item_ipv4 ipv4;
>> union {
>> struct rte_flow_item_tcp tcp;
>> struct rte_flow_item_udp udp;
>> struct rte_flow_item_sctp sctp;
>> struct rte_flow_item_icmp icmp;
>> };
>> } spec, mask;
>> uint8_t *ipv4_next_proto_mask = &mask.ipv4.hdr.next_proto_id;
>>
>> memset(&spec, 0, sizeof spec);
>> memset(&mask, 0, sizeof mask);
>>
>>
>> Ethernet is a common case, userspace datapath always has exact match on
>> vlan,
>> IPv4 is also the common case, I think. So current code in most cases we'll
>> not
>> call memset only for few of L4 protocols which are in the union here and
>> does not eat extra memory.
>> Anyway we're not on a very hot path.
>>
>> With that you'll be able to easily convert L4 proto checking 'if's to a
>> single
>> 'switch (proto)' statement and also clear the *ipv4_next_proto_mask
>> unconditionally.
>>
>> What do you think ?
>
> I fully agree and if you can have a separate patch it would be great.
OK. Good.
I'll prepare the patch as soon as this one will be merged.
>
>>
>> You could incorporate these changes to this patch or I could prepare the
>> separate patch on top of it.
>>
>> Anyway, for the current version:
>>
>> Acked-by: Ilya Maximets <i.maxim...@samsung.com>
>>
>
> Thanks!
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
