Re: [PATCH RFC net-next 07/13] selftests: forwarding: new test, verify bridge flood flags

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Apr 12, 2022 at 09:55:31AM +0200, Joachim Wiberg wrote:
> On Mon, Apr 11, 2022 at 20:21, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > On Mon, Apr 11, 2022 at 03:38:31PM +0200, Joachim Wiberg wrote:
> >> +# Verify per-port flood control flags of unknown BUM traffic.
> >> +#
> >> +#                     br0
> >> +#                    /   \
> >> +#                  h1     h2
> >
> > I think the picture is slightly inaccurate. From it I understand that h1
> > and h2 are bridge ports, but they are stations attached to the real
> > bridge ports, swp1 and swp2. Maybe it would be good to draw all interfaces.
> 
> Hmm, yeah either that or drop it entirely.  I sort of assumed everyone
> knew about the h<-[veth]->swp (or actual cable) setup, but you're right
> this is a bit unclear.  Me and Tobias have internally used h<-->p (for
> host<-->bridge-port) and other similar nomenclature.  Finding a good
> name that fits easily, and is still readable, in ASCII drawings is hard.
> I'll give it a go in the next drop, thanks!

I wasn't thinking of anything too fancy, this would do I guess.

             br0
            /   \
 h1 --- swp1     swp2 --- h2

> > Also, to be honest, a generic name like "ports" is hard to digest,
> > especially since you have another generic variable name "iface".
> > Maybe "brports" and "station" is a little bit more specific?
> 
> Is there a common naming standard between bridge tests, or is it more
> important to be consistent the test overview (test heading w/ picture)?
> 
> Anyway, I'll have a look at the naming for the next drop.

Even if there is a common naming standard in the selftests I wouldn't
know it. I just found the naming here to be vague enough that it is
confusing. If there are other examples of "port" + "iface" please feel
free to disregard.

> >> +declare -A flag1=([$swp1]=off [$swp2]=off [br0]=off)
> >> +declare -A flag2=([$swp1]=off [$swp2]=on  [br0]=off)
> >> +declare -A flag3=([$swp1]=off [$swp2]=on  [br0]=on )
> >> +declare -A flag4=([$swp1]=off [$swp2]=off [br0]=on )
> > If it's not too much, maybe these could be called "flags_pass1", etc.
> > Again, it was a bit hard to digest on first read.
> 
> More like flags_pass_fail, but since its the flooding flags, maybe
> flood_patternN would be better?

This works.