Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

Petr Vorel
2023-02-08T14:50:46Z

> Hi!
> > > What I didn't get here is why do we put the code to detect IPV6 into two
> > > different functions and then call it twice nearly from the same place in
> > > the library. Is there a reason why can't we put the sysctl check right
> > > after the proc check in the current ipv6 detection function?

> > Well, the old check (cmdline check for ipv6.disable=1) must be run before
> > creating interfaces. But the new sysctl based tests test interfaces
> > (${L,R}HOST_IFACES). Technically it could be run just after ipv6.disable=1
> > check, because it's needed only for the non-default ssh based two host setup [1].
> > But ${L,R}HOST_IFACES initialization:

> > export LHOST_IFACES="${LHOST_IFACES:-eth0}"
> > export RHOST_IFACES="${RHOST_IFACES:-eth0}"

> > would have to be moved before this check and run only for ssh based test
> > (the default netns based single host will create interfaces in init_ltp_netspace()
> > and for them ipv6.disable=1 check is good enough). I wanted to keep variables in
> > single place, but I'm ok to add this exception. Or we can keep variables where
> > they are and run this check on ${LHOST_IFACES:-} and ${RHOST_IFACES:-} (i.e.
> > only if these interfaces are set externally - which should be only on ssh two
> > host based setup).

> > I also thought that having to run this new check separately could be useful
> > (setup can change any time). But as nobody required that /complained it's
> > needed, let's ignore this.

> Ah, it's more complicated than a single switch, there is actually a
> master swtich, default settings and per interface settings.

> i.e. /proc/sys/net/ipv6/conf/default/disable_ipv6
> /proc/sys/net/ipv6/conf/all/disable_ipv6
> /proc/sys/net/ipv6/conf/${IFACE}/disable_ipv6

> Since the deafult value is I suppose inherited by newly added interfaces
> we have to check if ipv6 is disabled by sysctl in the case of network
> namespaces as well, right? Technically we could probably check the
> default/disable_ipv6 before we attempt to create new interfaces, but I
> guess that we would rather want to have the same code both for ssh and
> namespaces.

Good catch, yes, conf/default/disable_ipv6 is inherited, thus
mandatory for check altogether with conf/${IFACE}/disable_ipv6
(for ${LHOST_IFACES:-} and ${RHOST_IFACES:-} - ssh).
We can ignore conf/all/disable_ipv6 [1]:

Reading this value does not have any particular meaning. It does not say
whether IPv6 support is enabled or disabled. Returned value can be 1 also in
the case when some interface has disable_ipv6 set to 0 and has configured
IPv6 addresses.

I'd add this check before calling init_ltp_netspace(). That expects:
1) nobody changes this setup (one day we may want to test this functionality,
thus even LTP itself can change this setup) 2) any NIC without IPv6 means no
IPv6 testing, right?

BTW at least network tests which use virt_lib.sh requires IPv6 for IPv4 tests.
ip6_virt_remote="$(TST_IPV6=6 tst_ipaddr_un rhost)"
which is then used in virt_setup():

# ./macsec02.sh
macsec02 1 TINFO: setup IPsec transport/esp_aead des3_ede
macsec02 1 TINFO: setup local macsec with 'icvlen 16 encodingsa 0 replay on window 300 encrypt on protect on'
macsec02 1 TINFO: setup rhost macsec with 'icvlen 16 encodingsa 0 replay on window 300 encrypt on protect on'
RTNETLINK answers: Operation not supported
macsec02 1 TBROK: ip addr add ::2/64 dev ltp_v0 nodad failed

# ./vlan03.sh
RTNETLINK answers: Operation not supported
vlan03 1 TBROK: ip addr add ::2/64 dev ltp_v0 nodad failed

These tests would have to be either split to IPv4 (default) and IPv6 (-6)
or entirely depend on IPv6 (obviously first is better, but more work).

Kind regards,
Petr

[1] https://docs.kernel.org/networking/ip-sysctl.html