[ovs-dev] [PATCH] odp-execute: Extract l2 padding while executing check_pkt_len action.

Dumitru Ceara dceara at redhat.com
Thu Jun 11 10:16:21 UTC 2020


On 6/11/20 12:12 PM, Ilya Maximets wrote:
> On 6/11/20 12:02 PM, Ilya Maximets wrote:
>> If dp-packet contains l2 padding its size will be larger than the
>> actual size of a payload and action will work incorrectly.
>>
>> Padding could be set during miniflow_extract() if detected.
>>
>> CC: Numan Siddique <nusiddiq at redhat.com>
>> Fixes: 5b34f8fc3b38 ("Add a new OVS action check_pkt_larger")
>> Reported-by: Miroslav Kubiczek <miroslav.kubiczek at adaptivemobile.com>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/050157.html
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
>>
>> I didn't test this, only checked existing unit tests.
>>
>> Miroslav, could you, please, check if this works in your case?
>>
>> It might be also good to write some unit test for this.
>>
>>  lib/odp-execute.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
>> index 6018e378a..548083cef 100644
>> --- a/lib/odp-execute.c
>> +++ b/lib/odp-execute.c
>> @@ -761,10 +761,10 @@ odp_execute_check_pkt_len(void *dp, struct dp_packet *packet, bool steal,
>>  
>>      const struct nlattr *a;
>>      struct dp_packet_batch pb;
>> +    uint32_t size = dp_packet_size(packet) - dp_packet_l2_pad_size(packet);
> 
> We might also need to subtract dp_packet_get_cutlen() or use
> dp_packet_get_send_len() instead of dp_packet_size() for the
> same purpose, otherwise cutlen + check_pkt_len might not work
> as expected.
> 

I only had a quick glance at this and I'm not that familiar to the code
base but the patch looks good to me (with dp_packet_get_send_len()
instead of dp_packet_size()).

Regards,
Dumitru

>>  
>>      a = attrs[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN];
>> -    bool is_greater = dp_packet_size(packet) > nl_attr_get_u16(a);
>> -    if (is_greater) {
>> +    if (size > nl_attr_get_u16(a)) {
>>          a = attrs[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER];
>>      } else {
>>          a = attrs[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL];
>>
> 



More information about the dev mailing list