On 5/26/20 3:46 PM, Eli Britstein wrote:
> 
> On 5/19/2020 5:26 PM, Eli Britstein wrote:
>>
>> On 5/19/2020 4:41 PM, Sriharsha Basavapatna wrote:
>>> On Mon, May 18, 2020 at 9:10 PM Eli Britstein <[email protected]> wrote:
>>>> The function of adding patterns by requested matches checks that it
>>>> consumed all the required matches, and err if not. This nullify the
>>>> purpose of the validation function. Future supported matches will only
>>>> change the pattern parsing code.
>>>>
>>>> Signed-off-by: Eli Britstein <[email protected]>
>>>> Reviewed-by: Roni Bar Yanai <[email protected]>
>>>> ---
>>>>   lib/netdev-offload-dpdk.c | 133 
>>>> ++++++++++++++++++----------------------------
>>>>   1 file changed, 51 insertions(+), 82 deletions(-)
>>>>
>>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>>>> index f8c46bbaa..84bbe29e7 100644
>>>> --- a/lib/netdev-offload-dpdk.c
>>>> +++ b/lib/netdev-offload-dpdk.c
>>>> @@ -559,10 +559,22 @@ free_flow_actions(struct flow_actions *actions)
>>>>
>>>>   static int
>>>>   parse_flow_match(struct flow_patterns *patterns,
>>>> -                 const struct match *match)
>>>> +                 struct match *match)
>>>>   {
>>>>       uint8_t *next_proto_mask = NULL;
>>>> +    struct flow *consumed_masks;
>>>>       uint8_t proto = 0;
>>>> +    int ret = 0;
>>>> +
>>>> +    consumed_masks = &match->wc.masks;
>>>> +
>>>> +    memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
>>>> +    if (match->flow.recirc_id != 0) {
>>>> +        ret = -1;
>>>> +        goto out;
>>>> +    }
>>>> +    consumed_masks->recirc_id = 0;
>>>> +    consumed_masks->packet_type = 0;
>>>>
>>>>       /* Eth */
>>>>       if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
>>>> @@ -580,6 +592,9 @@ parse_flow_match(struct flow_patterns *patterns,
>>>>           memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
>>>>           mask->type = match->wc.masks.dl_type;
>>>>
>>>> +        memset(&consumed_masks->dl_dst, 0, sizeof consumed_masks->dl_dst);
>>>> +        memset(&consumed_masks->dl_src, 0, sizeof consumed_masks->dl_src);
>>>> +
>>>>           add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
>>>>       } else {
>>>>           /*
>>>> @@ -591,6 +606,7 @@ parse_flow_match(struct flow_patterns *patterns,
>>>>            */
>>>>           add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
>>>>       }
>>>> +    consumed_masks->dl_type = 0;
>>> Why not set the dl_type also under the if() condition above, where
>>> dl_src an dl_dst consumed masks are being set ? Similarly, vlan
>>> consumed mask below.
>> OVS always matches on dl_type (same for the below of VLAN). We must clean it 
>> in any case in order not to fail.
> That's right. It's a bug here from upstream. dl_type should be handled even 
> if no matches on src/dst. I'll add a commit to fix it in v2.

Is it something related:
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to