[Fuego] change vlan id maximum to 4094 for vlan01

Bird, Tim Tim.Bird at sony.com
Wed Sep 23 23:06:24 UTC 2020


Sorry for the slow response.  This took quite a bit of time to understand.
I'm not familiar with these LTP networking tests, so I had to do some
research.

> -----Original Message-----
> From: qiutt
> 
> details: for vlan01 in net.features, create a vlan interface in the vlan id range (maximum 4095),
> and then delete the created vlan interface in turn.
> but VLAN identifier (VID): a 12-bit field specifying the VLAN to
> which the frame belongs. The hexadecimal values of 0x000 and 0xFFF are
> reserved. All other values may be used as VLAN identifiers, allowing up
> to 4,094 VLANs.
> refer to
> https://networkengineering.stackexchange.com/questions/24404/vlan-0-1-and-4095-are-reserved-what-are-they-reserved-for

This description was OK, but could be improved.  It looks like it's mostly
a quote from the stackexchange answer, which is a quote from Wikipedia.

This is useful to let me know that the max should be 4094.  However, the commit
message doesn't help me understand why this patch should be applied.

When is a problem with the vlan id encountered?  Did you hit this while testing at Fujitsu?

I dug into when start_id and NS_TIMES get set (both of which affect the value used
to set max, and it's unclear when they would cause the vlan id to overflow the allowed range.
As far as I can see, start_id will normally be set to 1, and NS_TIMES will normally be set to 10.
In your testing, how did start_id or NS_TIMES get high enough to trigger an overflow?

Under what conditions would these result in a max that was > 4094?

> 
> Signed-off-by: qiutt <qiutt at cn.fujitsu.com>
> ---
>  tests/Functional.LTP/fuego_test.sh                  |  2 ++
>  tests/Functional.LTP/ltp-network-net.features.patch | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 tests/Functional.LTP/ltp-network-net.features.patch
> 
> diff --git a/tests/Functional.LTP/fuego_test.sh b/tests/Functional.LTP/fuego_test.sh
> index c4c8129..4bd592a 100755
> --- a/tests/Functional.LTP/fuego_test.sh
> +++ b/tests/Functional.LTP/fuego_test.sh
> @@ -358,6 +358,8 @@ function test_build {
>          skip_if_config_unavailable "HAVE_NUMA" "get_mempolicy01 mbind01 migrate_pages01 migrate_pages02 move_pages01
> move_pages02 move_pages03 move_pages04 move_pages05 move_pages06 move_pages07 move_pages08 move_pages09
> move_pages10 move_pages11" "No NUMA support"
>          skip_if_config_unavailable "HAVE_MODIFY_LDT" "modify_ldt01 modify_ldt02 modify_ldt03" "Add modify_ldt support"
> 
> +        patch -p0 < ${TEST_HOME}/ltp-network-net.features.patch
> +
I'm OK taking an LTP patch, if it fixes a problem.  However, the patch or fix should also be
submitted to upstream to make sure that the fix will be applied for others in the long run.

Has this patch been sent to LTP mainline?

>          # save build results in build.log
>          # Typical build error:
>          #   undefined reference to `io_submit'
> diff --git a/tests/Functional.LTP/ltp-network-net.features.patch b/tests/Functional.LTP/ltp-network-net.features.patch
> new file mode 100644
> index 0000000..6bc80ff
> --- /dev/null
> +++ b/tests/Functional.LTP/ltp-network-net.features.patch
> @@ -0,0 +1,20 @@
> +--- testcases/network/virt/virt_lib.sh	2020-09-08 16:30:55.178336290 +0800
> ++++ testcases/network/virt/virt_lib.sh	2020-09-08 16:30:09.469282575 +0800
> +@@ -144,7 +144,7 @@ virt_add_rhost()
> + virt_multiple_add_test()
> + {
> + 	local opt="$@"
> +-	local max=$(($start_id + $NS_TIMES - 1))
> ++	local max=$((($start_id + $NS_TIMES - 1) > 4094 ? 4094 : ($start_id + $NS_TIMES - 1)))
This looks OK.

> +
> + 	tst_resm TINFO "add $NS_TIMES $virt_type, then delete"
> +
> +@@ -165,7 +165,7 @@ virt_multiple_add_test()
> + virt_add_delete_test()
> + {
> + 	local opt="$@"
> +-	local max=$(($start_id + $NS_TIMES - 1))
> ++	local max=$((($start_id + $NS_TIMES - 1) > 4094 ? 4094 : ($start_id + $NS_TIMES - 1)))
This doesn't look right.

In the tarball (ltp-full-20180118.tar.bz2) and in the current LTP source on github,
these line looks like this in virt_lib.sh

virt_add_delete_test()
{
    local opt="$@"
    local max=$(($NS_TIMES - 1))

So the patch is going to fail to apply to either the tarball LTP source or the github source.
(the line being changed doesn't match the one in the patch)

Can you please explain where you got a virt_lib.sh from, that had the lines in the patch?

> +
> + 	tst_resm TINFO "add/del $virt_type $NS_TIMES times"
> +
> --
> 2.11.0

I appreciate this work.  I did some testing, and when start_id or NS_TIMES is high enough,
the overflow of the valid id range does cause problems.  However, since the patch doesn't
apply to any of the LTP versions I am aware of, it doesn't seem like a fix that would be
helpful.

Please answer my questions and maybe we can resolve the issues.

Regards,
 -- Tim



More information about the Fuego mailing list