Re: [ovs-dev] [PATCH] tests/flowgen: Fix length field of 802.2 data link header.

[Ilya Maximets]

On 11/24/21 19:32, Paolo Valerio wrote:
> Hello Ilya,
> 
> Ilya Maximets  writes:
> 
>> Length in Data Link Header for these packets should not include
>> source and destination MACs or the length field itself.
>>
>> Therefore, it should be 14 bytes less, otherwise other network
>> tools like wireshark complains:
>>
>>   Expert Info (Error/Malformed):
>> Length field value goes past the end of the payload
>>
>> Additionally fixing the printing of the packet/flow configuration,
>> as it currently prints '%s=%s' strings without any real data.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
> 
> The patch LGTM and works as expected.
> Do you think it makes sense to include a 'Fixes' tag?

I don't know perl enough to figure out if the issue existed
in the original perl script and I'm too lazy to actually test
that.  The issue exists on all supported branches and it's also
the issue with tests and not with the OVS itself, so the 'Fixes'
tag is not very important.

> 
> In any case:
> 
> Acked-by: Paolo Valerio 
> 

Thanks!  Applied and backported down to 2.13.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests/flowgen: Fix length field of 802.2 data link header.

[Paolo Valerio]

Hello Ilya,

Ilya Maximets  writes:

> Length in Data Link Header for these packets should not include
> source and destination MACs or the length field itself.
>
> Therefore, it should be 14 bytes less, otherwise other network
> tools like wireshark complains:
>
>   Expert Info (Error/Malformed):
> Length field value goes past the end of the payload
>
> Additionally fixing the printing of the packet/flow configuration,
> as it currently prints '%s=%s' strings without any real data.
>
> Signed-off-by: Ilya Maximets 
> ---

The patch LGTM and works as expected.
Do you think it makes sense to include a 'Fixes' tag?

In any case:

Acked-by: Paolo Valerio 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests/flowgen: Fix length field of 802.2 data link header.

[Ilya Maximets]

Length in Data Link Header for these packets should not include
source and destination MACs or the length field itself.

Therefore, it should be 14 bytes less, otherwise other network
tools like wireshark complains:

  Expert Info (Error/Malformed):
Length field value goes past the end of the payload

Additionally fixing the printing of the packet/flow configuration,
as it currently prints '%s=%s' strings without any real data.

Signed-off-by: Ilya Maximets 
---
 tests/flowgen.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/flowgen.py b/tests/flowgen.py
index 7ef32d13c..a6c0024bf 100755
--- a/tests/flowgen.py
+++ b/tests/flowgen.py
@@ -166,15 +166,15 @@ def output(attrs):
 ip = ip[:2] + struct.pack('>H', len(ip)) + ip[4:]
 packet += ip
 if attrs['DL_HEADER'].startswith('802.2'):
-packet_len = len(packet)
+packet_len = len(packet) - 14
 if flow['DL_VLAN'] != 0x:
 packet_len -= 4
 packet = (packet[:len_ofs]
   + struct.pack('>H', packet_len)
   + packet[len_ofs + 2:])
 
-print(' '.join(['%s=%s' for k, v in attrs.items()]))
-print(' '.join(['%s=%s' for k, v in flow.items()]))
+print(' '.join(['%s=%s' % (k, v) for k, v in attrs.items()]))
+print(' '.join(['%s=%s' % (k, v) for k, v in flow.items()]))
 print()
 
 flows.write(struct.pack('>LH',
-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev