Re: [PATCH v2 net-next] selftests: forwarding: add Per-Stream Filtering and Policing test for Ocelot

[Ferenc Fejes <ferenc.fejes@ericsson.com>]

Hi!

On 2022. 05. 01. 13:29, Vladimir Oltean wrote:
> The Felix VSC9959 switch in NXP LS1028A supports the tc-gate action
> which enforced time-based access control per stream. A stream as seen by
> this switch is identified by {MAC DA, VID}.
>
> We use the standard forwarding selftest topology with 2 host interfaces
> and 2 switch interfaces. The host ports must require timestamping non-IP
> packets and supporting tc-etf offload, for isochron to work. The
> isochron program monitors network sync status (ptp4l, phc2sys) and
> deterministically transmits packets to the switch such that the tc-gate
> action either (a) always accepts them based on its schedule, or
> (b) always drops them.
>
> I tried to keep as much of the logic that isn't specific to the NXP
> LS1028A in a new tsn_lib.sh, for future reuse. This covers
> synchronization using ptp4l and phc2sys, and isochron.
>
> The cycle-time chosen for this selftest isn't particularly impressive
> (and the focus is the functionality of the switch), but I didn't really
> know what to do better, considering that it will mostly be run during
> debugging sessions, various kernel bloatware would be enabled, like
> lockdep, KASAN, etc, and we certainly can't run any races with those on.
>
> I tried to look through the kselftest framework for other real time
> applications and didn't really find any, so I'm not sure how better to
> prepare the environment in case we want to go for a lower cycle time.
> At the moment, the only thing the selftest is ensuring is that dynamic
> frequency scaling is disabled on the CPU that isochron runs on. It would
> probably be useful to have a blacklist of kernel config options (checked
> through zcat /proc/config.gz) and some cyclictest scripts to run
> beforehand, but I saw none of those.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>
> +switch_create()
> +{
> +	local h2_mac_addr=$(mac_get $h2)
> +
> +	ip link set ${swp1} up
> +	ip link set ${swp2} up
> +
> +	ip link add br0 type bridge vlan_filtering 1
> +	ip link set ${swp1} master br0
> +	ip link set ${swp2} master br0
> +	ip link set br0 up
> +
> +	bridge vlan add dev ${swp2} vid ${STREAM_VID}
> +	bridge vlan add dev ${swp1} vid ${STREAM_VID}
> +	# PSFP on Ocelot requires the filter to also be added to the bridge
> +	# FDB, and not be removed
> +	bridge fdb add dev ${swp2} \
> +		${h2_mac_addr} vlan ${STREAM_VID} static master
> +
> +	psfp_chain_create ${swp1}
> +
> +	tc filter add dev ${swp1} ingress chain $(PSFP) pref 1 \
> +		protocol 802.1Q flower skip_sw \
> +		dst_mac ${h2_mac_addr} vlan_id ${STREAM_VID} \
> +		action gate base-time 0.000000000 \
> +		sched-entry OPEN  ${GATE_DURATION_NS} -1 -1 \
> +		sched-entry CLOSE ${GATE_DURATION_NS} -1 -1

I know that might be little bit off-topic here, but the current 
implementation of the act_gate does nothing with the IPV value [0] even 
if the user set it to non -1.
IMO this IPV value should be carried through in the tcf_gate struct [1] 
as something like a "current_ipv" member or so. Then this value can be 
applied in the tcf_gate_act function to the skb->priority.

Background story: I tried to combine gate and taprio (802.1Qci and Qbv) 
to achieve 802.1Qch operation (which is really just a coordinated config 
of those two) but without the IPV (should by set by the ingress port) we 
have no way to carry the gating info to the taprio, and as a result its 
just sending every packet with the default priority, no matter how we 
open/close the gate at the ingress.

[0] 
https://elixir.bootlin.com/linux/v5.18-rc5/source/include/net/tc_act/tc_gate.h#L21
[1] 
https://elixir.bootlin.com/linux/v5.18-rc5/source/include/net/tc_act/tc_gate.h#L40
[2] 
https://elixir.bootlin.com/linux/v5.18-rc5/source/net/sched/act_gate.c#L117

> +}
Ferenc