netdev - Re: [RFC PATCH] batman-adv: Reserve needed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <73D4D29E-C26E-471B-91CC-442B046F08AE@exaring.de>
Date:   Thu, 26 Nov 2020 08:58:45 +0000
From:   Annika Wickert <annika.wickert@...ring.de>
To:     Sven Eckelmann <sven@...fation.org>,
        "b.a.t.m.a.n@...ts.open-mesh.org" <b.a.t.m.a.n@...ts.open-mesh.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>,
        "dev@...nvswitch.org" <dev@...nvswitch.org>
Subject: Re: [RFC PATCH] batman-adv: Reserve needed_headroom for fragments

Hi,

    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);

Will add this thx.

    >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?

BATMAN already has the vxlan interface as hardif here is the script I use to generate the config:

ip link add mesh-vpn type vxlan id 4831583 local fe80::2e0:2fff:fe18:dc2f remote fe80::281:8eff:fef0:73aa  dstport 8472 dev wg-uplink
ip link del bat-welt 
rmmod batman-adv
modprobe batman-adv
 batctl ra BATMAN_V
 batctl meshif bat-welt interface add mesh-vpn
 ip link set up bat-welt
 ip link set dev bat-welt master br-welt

   > 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.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.

Which should already include the headroom needed by vxlan as it's already present as hardif. 

    >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.

    >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)?

  The vxlan interface was added first. So it should propagate it?

    >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?


As the setup is always doing the pskb_expand_head() it should be possible to test this.

Best Annika