Message-ID: <5658440.UjTJXf6HLC@sven-edge>
Date:   Thu, 26 Nov 2020 09:21:58 +0100
From:   Sven Eckelmann <sven@...fation.org>
To:     "b.a.t.m.a.n@...ts.open-mesh.org" <b.a.t.m.a.n@...ts.open-mesh.org>,
        Annika Wickert <annika.wickert@...ring.de>,
        netdev@...r.kernel.org, bridge@...ts.linux-foundation.org,
        dev@...nvswitch.org
Subject: Re: [RFC PATCH] batman-adv: Reserve needed_headroom for fragments

Hi,

I find your output slightly confusing. Maybe you can change your printk stuff 
to something more like:

  printk("%s %s:%u max_headroom %u\n", __FILE__, __func__, __LINE__, max_headroom);

On Thursday, 26 November 2020 00:14:35 CET Annika Wickert wrote:
> This is what I get from the bridge when bat0 is enslaved with the vxlan interface as member of batman ( https://elixir.bootlin.com/linux/latest/source/net/bridge/br_if.c#L311 )
> [   36.959547] Bridge firewalling registered
> [  522.221767] SKB Bridge br_if.c: max_headroom 0
> [  522.221781] SKB Bridge br_if.c: new_hr 0
> [  627.186129] SKB Bridge br_if.c: max_headroom 0
> [  627.186139] SKB Bridge br_if.c: new_hr 0
> [  627.616650] SKB Bridge br_if.c: new_hr 102

When is this shown? Does the batadv interface already have its hardif (slave) 
interfaces attached at that point? And did the vxlan report the correct 
needed_headroom to batadv before you've tried to attach the batadv interface 
to the bridge?

Because the bridge can also only change its needed_headroom on interface add 
or delete.

> Also BATMAN reports itself when initialized and seems not to propagate stuff up the stack on change?: (https://github.com/open-mesh-mirror/batman-adv/blob/master/net/batman-adv/hard-interface.c#L555  )
> [ 3350.212094] SKB hard-interface.h: soft_iface->needed_tailroom) 0
> [3350.212105] SKB hard-interface.h: soft_iface->needed_headroom) 358
> [ 3350.212116] SKB hard-interface.h: lower_headroom 70
> [ 3350.212126] SKB hard-interface.h: needed_headroom 102

Afaik, it is "propagating" its stuff by adjusting its own needed_headroom/
tailroom at this point. But there is no way to notify that the headroom/
tailroom was changed and the upper layers should recalculate it.

If you need something like this then we might to have a new 
NETDEV_RESERVED_SPACE_CHANGE (or a better name OR maybe use a netdev_cmd with 
a similar meaning). And then call this whenever the needed_headroom/
tailroom/... of an interface changes during its lifetime. And bridge/batman-
adv/ovs/... have to check the headroom in their notifier_call again when they 
receive this event.

> Also added some debugging to Fragmentation.c in BATMAN after the patch: 
> Nov 25 17:48:26 raspi-1gb.awlnx.space kernel: SKB Fragmentation.c: ll_reserved 96
> Nov 25 17:48:26 raspi-1gb.awlnx.space kernel: SKB Fragmentation.c: skb->len 762
> Nov 25 17:48:26 raspi-1gb.awlnx.space kernel: SKB Fragmentation.c: header_size 20
> Nov 25 17:48:26 raspi-1gb.awlnx.space kernel: SKB Fragmentation.c: fragment_size 762
> Nov 25 17:48:26 raspi-1gb.awlnx.space kernel: SKB Fragmentation.c: ll_reserved 96
> 
> At the same time the VXLAN interface which is transported over Wireguard reports this (https://elixir.bootlin.com/linux/latest/source/drivers/net/vxlan.c#L2352 )
> [  567.515778] SKB VXLAN vxlan.c: min_headroom 200
> [  567.515792] SKB VXLAN vxlan.c: dst->header_len 0
> [  567.515805] SKB VXLAN vxlan.c: VXLAN_HLEN 16
> [  567.515819] SKB VXLAN vxlan.c: LL_RESERVED_SPACE(dst->dev) 144
> [  567.515831] SKB VXLAN vxlan.c: iphdr_len 40
> 
> So in my opinion the needed headroom reported by batman is wrong by maybe 200 ? As the min_headroom of vxlan seems to be 200 but BATMAN reports 102 up the stack to the bridge.

Could it be that the vxlan didn't had the correct needed_headroom when you've 
added it to you batadv interface? Or that the vxlan interface didn't set the 
correct needed_headroom for its lower_dev (see vxlan_config_apply)?



If you have the "slow" setup, can you please do following steps:

* keep vxlan as is (I hope you specify a fixed lowerdev)

  - but try to print the needed headroom in vxlan_config_apply and compare it 
    to the ones from vxlan_build_skb

* remove the vxlan from your batadv interface
* add your vxlan again from the batadv interface

  - check if the headroom numbers are now looking better in 
    batadv_hardif_recalc_extra_skbroom

* remove batadv interface from the bridge
* add your batadv interface again to the bridge

  - is update_headroom() now using the correct headroom information?

> If you need any more input we are happy to help. Because the actual performance with running batman over vxlan is really bad. 
> We have some figures here: https://gist.github.com/fadenb/9705059cf17eddf60e744e95bf926f05

Kind regards,
	Sven
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists