[ovs-dev] [PATCH 2/3] tc: Split IPs and transport layer ports unions in flower struct

[Paul Blakey]

On 30/07/2017 10:22, Roi Dayan wrote:
> 
> 
> On 28/07/2017 15:49, Simon Horman wrote:
>> On Thu, Jul 27, 2017 at 01:19:59PM +0300, Roi Dayan wrote:
>>> From: Paul Blakey <paulb at mellanox.com>
>>>
>>> Split dst/src_port and ipv4/ipv6 union so we can
>>> distingush them easily for later features.
>>
>> The implications of this change on the size of struct tc_flower_key
>> seem somewhat undesirable to me. Perhaps there is another way to
>> distinguish between protocols?
>>
> 
> the intention here was to ease implementation later of ovs action set
> offloading and make it generic.
> we'll take a look again on this and update.
> as a general question why the size change of tc_flower_key matters in
> this case?
> 
>>> Signed-off-by: Paul Blakey <paulb at mellanox.com>
>>> Reviewed-by: Roi Dayan <roid at mellanox.com>

Hi, Yes and to add to that,
We split those so in the header rewrite (supporting ovs action set using 
tc pedit action) patches that will follow, we can use a map that 
translates flower key offsets to tc action pedit offsets.
Without this change we can't know just by looking at offsetof(struct 
tc_flower_key.ipv4.ipv4_src) if it was set because ipv6_src was set 
(union) or ipv4_src was set, we would have to look at the protocol as 
well. This can be done in a map by specifying the protocol but it 
complicates things.
Besides flower struct is allocated on the stack in 
netdev_tc_offloads_flow_put()



We have a map like so:
+struct tc_pedit_key_ex {
+    enum pedit_header_type htype;
+    enum pedit_cmd cmd;
+};
+
+struct flower_key_to_pedit {
+    enum pedit_header_type htype;
+    int offset;
+    int size;
+};
+
+static struct flower_key_to_pedit flower_pedit_map[] = {
+    [offsetof(struct tc_flower_key, ipv4.ipv4_src)] = {
+        TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+        12,
+        MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_src)
+    },
+    [offsetof(struct tc_flower_key, ipv4.ipv4_dst)] = {
+        TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+        16,
+        MEMBER_SIZEOF(struct tc_flower_key, ipv4.ipv4_dst)
+    },
+    [offsetof(struct tc_flower_key, ipv4.rewrite_ttl)] = {
+        TCA_PEDIT_KEY_EX_HDR_TYPE_IP4,
+        8,
+        MEMBER_SIZEOF(struct tc_flower_key, ipv4.rewrite_ttl)
+    },
....