14 Jun, 2017
1 commit
-
Simon Wunderlich says:
====================
Here are two batman-adv bugfixes:- fix rx packet counters for local ARP replies, by Sven Eckelmann
- fix memory leaks for unicast packetes received from another gateway
in bridge loop avoidance, by Andreas Pape
====================Signed-off-by: David S. Miller
08 Jun, 2017
1 commit
-
Network devices can allocate reasources and private memory using
netdev_ops->ndo_init(). However, the release of these resources
can occur in one of two different places.Either netdev_ops->ndo_uninit() or netdev->destructor().
The decision of which operation frees the resources depends upon
whether it is necessary for all netdev refs to be released before it
is safe to perform the freeing.netdev_ops->ndo_uninit() presumably can occur right after the
NETDEV_UNREGISTER notifier completes and the unicast and multicast
address lists are flushed.netdev->destructor(), on the other hand, does not run until the
netdev references all go away.Further complicating the situation is that netdev->destructor()
almost universally does also a free_netdev().This creates a problem for the logic in register_netdevice().
Because all callers of register_netdevice() manage the freeing
of the netdev, and invoke free_netdev(dev) if register_netdevice()
fails.If netdev_ops->ndo_init() succeeds, but something else fails inside
of register_netdevice(), it does call ndo_ops->ndo_uninit(). But
it is not able to invoke netdev->destructor().This is because netdev->destructor() will do a free_netdev() and
then the caller of register_netdevice() will do the same.However, this means that the resources that would normally be released
by netdev->destructor() will not be.Over the years drivers have added local hacks to deal with this, by
invoking their destructor parts by hand when register_netdevice()
fails.Many drivers do not try to deal with this, and instead we have leaks.
Let's close this hole by formalizing the distinction between what
private things need to be freed up by netdev->destructor() and whether
the driver needs unregister_netdevice() to perform the free_netdev().netdev->priv_destructor() performs all actions to free up the private
resources that used to be freed by netdev->destructor(), except for
free_netdev().netdev->needs_free_netdev is a boolean that indicates whether
free_netdev() should be done at the end of unregister_netdevice().Now, register_netdevice() can sanely release all resources after
ndo_ops->ndo_init() succeeds, by invoking both ndo_ops->ndo_uninit()
and netdev->priv_destructor().And at the end of unregister_netdevice(), we invoke
netdev->priv_destructor() and optionally call free_netdev().Signed-off-by: David S. Miller
19 May, 2017
2 commits
-
The skb must be released in the receive handler since b91a2543b4c1
("batman-adv: Consume skb in receive handlers"). Just returning NET_RX_DROP
will no longer automatically free the memory. This results in memory leaks
when unicast packets from other backbones must be dropped because they
share a common backbone.Fixes: 9e794b6bf4a2 ("batman-adv: drop unicast packets from other backbone gw")
Signed-off-by: Andreas Pape
[sven@narfation.org: adjust commit message]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The stats are generated by batadv_interface_stats and must not be stored
directly in the net_device stats member variable. The batadv_priv
bat_counters information is assembled when ndo_get_stats is called. The
stats previously stored in net_device::stats is then overwritten.The batman-adv counters must therefore be increased when an ARP packet is
answered locally via the distributed arp table.Fixes: c384ea3ec930 ("batman-adv: Distributed ARP Table - add snooping functions for ARP messages")
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
07 Apr, 2017
1 commit
-
Simon Wunderlich says:
====================
This feature/cleanup patchset includes the following patches:- bump version strings, by Simon Wunderlich
- Code and Style cleanups, by Sven Eckelmann (5 patches)
- Remove an unneccessary memset, by Tobias Klauser
- DAT and BLA optimizations for various corner cases, by Andreas Pape
(5 patches)- forward/rebroadcast packet restructuring, by Linus Luessing
(2 patches)- ethtool cleanup and remove unncessary code, by Sven Eckelmann
(4 patches)- use net_device_stats from net_device instead of private copy,
by Tobias Klauser
====================Signed-off-by: David S. Miller
05 Apr, 2017
1 commit
-
Instead of using a private copy of struct net_device_stats in struct
batadv_priv, use stats from struct net_device.Signed-off-by: Tobias Klauser
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
03 Apr, 2017
4 commits
-
The ethtool code was spread in soft-interface.c. This makes reading the
code and working on it unnecessary complicated. Having everything in a
common place next to the other code which references it, makes it slightly
easier.Signed-off-by: Sven Eckelmann
Acked-by: Marek Lindner
Signed-off-by: Simon Wunderlich -
The .get_settings function pointer and the related API was deprecated.
Fortunately, batman-adv is a virtual interface and never provided any
useful information via .get_settings. The stub can therefore be
removed.This also avoids that incorrect information is shown in ethtool about the
batadv interface.Signed-off-by: Sven Eckelmann
Acked-by: Marek Lindner
Signed-off-by: Simon Wunderlich -
batadv devices don't support msglevel. The ethtool stubs therefore returned
that it isn't supported. But instead, the complete function can be dropped
to avoid that bogus values are shown in ethtool.Signed-off-by: Sven Eckelmann
Acked-by: Marek Lindner
Signed-off-by: Simon Wunderlich -
The ethtool_ops of batman-adv never contained more than a stub for the
get_link function pointer. It was always returning that a link exists even
when the devices was not yet up and therefore nothing resampling a link
could have been available.Instead use the ethtool helper which returns the current carrier state.
Signed-off-by: Sven Eckelmann
Acked-by: Marek Lindner
Signed-off-by: Simon Wunderlich
26 Mar, 2017
2 commits
-
This patch refactors the num_packets counter of a forw_packet in the
following three ways:1) Removed dual-use of forw_packet::num_packets:
-> now for aggregation purposes only
2) Using forw_packet::skb::cb::num_bcasts instead:
-> for easier access in aggregation code later
3) make access to num_bcasts private to batadv_forw_packet_*()Signed-off-by: Linus Lüssing
[sven@narfation.org: Change num_bcasts to unsigned]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
An skb is assigned to a forw_packet only once, shortly after the
forw_packet allocation.With this patch the assignment is moved into the this allocation
function.Signed-off-by: Linus Lüssing
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
22 Mar, 2017
5 commits
-
Consider the following situation which has been found in a test setup:
Gateway B has claimed client C and gateway A has the same backbone
network as B. C sends a broad- or multicast to B and directly after
this packet decides to send another packet to A due to a better TQ
value. B will forward the broad-/multicast into the backbone as it is
the responsible gw and after that A will claim C as it has been
chosen by C as the best gateway. If it now happens that A claims C
before it has received the broad-/multicast forwarded by B (due to
backbone topology or due to some delay in B when forwarding the
packet) we get a critical situation: in the current code A will
immediately unclaim C when receiving the multicast due to the
roaming client scenario although the position of C has not changed
in the mesh. If this happens the multi-/broadcast forwarded by B
will be sent back into the mesh by A and we have looping packets
until one of the gateways claims C again.
In order to prevent this, unclaiming of a client due to the roaming
client scenario is only done after a certain time is expired after
the last claim of the client. 100 ms are used here, which should be
slow enough for big backbones and slow gateways but fast enough not
to break the roaming client use case.Acked-by: Simon Wunderlich
Signed-off-by: Andreas Pape
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
Some of the bla debug messages are extended and additional messages are
added for easier bla debugging. Some debug messages introduced with the
dat changes in prior patches of this patch series have been changed to
be more compliant to other existing debug messages.Acked-by: Simon Wunderlich
Signed-off-by: Andreas Pape
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
Additional dropping of unicast packets received from another backbone gw if
the same backbone network before being forwarded to the same backbone again
is necessary. It was observed in a test setup that in rare cases these
frames lead to looping unicast traffic backbone->mesh->backbone.Signed-off-by: Andreas Pape
Acked-by: Simon Wunderlich
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
If none of the backbone gateways in a bla setup has already knowledge of
the mac address searched for in an incoming ARP request from the backbone
an address resolution via the DHT of DAT is started. The gateway can send
several ARP requests to different DHT nodes and therefore can get several
replies. This patch assures that not all of the possible ARP replies are
returned to the backbone by checking the local DAT cache of the gateway.
If there is an entry in the local cache the gateway has already learned
the requested address and there is no need to forward the additional reply
to the backbone.
Furthermore it is checked if this gateway has claimed the source of the ARP
reply and only forwards it to the backbone if it has claimed the source or
if there is no claim at all.Signed-off-by: Andreas Pape
Acked-by: Simon Wunderlich
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
If dat is enabled it must be made sure that only the backbone gw which has
claimed the remote destination for the ARP request answers the ARP request
directly if the MAC address is known due to the local dat table. This
prevents multiple ARP replies in a common backbone if more than one
gateway already knows the remote mac searched for in the ARP request.Signed-off-by: Andreas Pape
Acked-by: Simon Wunderlich
[sven@narfation.org: fix conflicts with current version]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
17 Mar, 2017
8 commits
-
The memory for netdev_priv is allocated using kzalloc in alloc_netdev
(or alloc_netdev_mq respectively) so there is no need to set it to 0
again.Signed-off-by: Tobias Klauser
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The name of the function might change in which these messages are printed.
It is therefore better to let the compiler handle the insertion of the
correct function name.Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The BATADV_PRINT_VID is not free of of possible side-effects. This can be
avoided when the the macro is converted to a simple inline function.Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
An argument of a macro should not be evaluated multiple times. Otherwise
embedded operations in these arguments will be executed multiple times.Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
It is not necessary to disable these code sections in case other kernel
features are disabled. Instead the IS_ENABLED tests can be added directly
in the code and the compiler can remove the unnecessary code parts during
its optimization run.Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
Signed-off-by: Simon Wunderlich
-
Simon Wunderlich says:
====================
Here are two batman-adv bugfixes:- Keep fragments equally sized, avoids some problems with too small fragments,
by Sven Eckelmann- Initialize gateway class correctly when BATMAN V is compiled in,
by Sven Eckelmann
====================Signed-off-by: David S. Miller
05 Mar, 2017
2 commits
-
The gateway selection class variable is shared between different algorithm
versions. But the interpretation of the content is algorithm specific. The
initialization is therefore also algorithm specific.But this was implemented incorrectly and the initialization for BATMAN_V
always overwrote the value previously written for BATMAN_IV. This could
only be avoided when BATMAN_V was disabled during compile time.Using a special batadv_algo hook for this initialization avoids this
problem.Fixes: 50164d8f500f ("batman-adv: B.A.T.M.A.N. V - implement GW selection logic")
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The batman-adv fragmentation packets have the design problem that they
cannot be refragmented and cannot handle padding by the underlying link.
The latter often leads to problems when networks are incorrectly configured
and don't use a common MTU.The sender could for example fragment a 1271 byte frame (plus external
ethernet header (14) and batadv unicast header (10)) to fit in a 1280 bytes
large MTU of the underlying link (max. 1294 byte frames). This would create
a 1294 bytes large frame (fragment 2) and a 55 bytes large frame
(fragment 1). The extra 54 bytes are the fragment header (20) added to each
fragment and the external ethernet header (14) for the second fragment.Let us assume that the next hop is then not able to transport 1294 bytes to
its next hop. The 1294 byte large frame will be dropped but the 55 bytes
large fragment will still be forwarded to its destination.Or let us assume that the underlying hardware requires that each frame has
a minimum size (e.g. 60 bytes). Then it will pad the 55 bytes frame to 60
bytes. The receiver of the 60 bytes frame will no longer be able to
correctly assemble the two frames together because it is not aware that 5
bytes of the 60 bytes frame are padding and don't belong to the reassembled
frame.This can partly be avoided by splitting frames more equally. In this
example, the 675 and 674 bytes large fragment frames could both potentially
reach its destination without being too large or too small.Reported-by: Martin Weinelt
Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu")
Signed-off-by: Sven Eckelmann
Acked-by: Linus Lüssing
Signed-off-by: Simon Wunderlich
03 Mar, 2017
2 commits
-
…kernel/git/jberg/mac80211
Johannes Berg says:
====================
This contains just the average.h change in order to get it
into the tree before adding new users through -next trees.
====================Signed-off-by: David S. Miller <davem@davemloft.net>
-
Simon Wunderlich says:
====================
Here are two batman-adv bugfixes:- fix a potential double free when fragment merges fail,
by Sven Eckelmann- fix failing tranmission of the 16th (last) fragment if that exists,
by Linus Lüssing
====================Signed-off-by: David S. Miller
02 Mar, 2017
1 commit
-
Declaring the factor is counter-intuitive, and people are prone
to using small(-ish) values even when that makes no sense.Change the DECLARE_EWMA() macro to take the fractional precision,
in bits, rather than a factor, and update all users.While at it, add some more documentation.
Acked-by: David S. Miller
Signed-off-by: Johannes Berg
22 Feb, 2017
2 commits
-
Trying to split and transmit a unicast packet in 16 parts will fail for
the final fragment: After having sent the 15th one with a frag_packet.no
index of 14, we will increase the the index to 15 - and return with an
error code immediately, even though one more fragment is due for
transmission and allowed.Fixing this issue by moving the check before incrementing the index.
While at it, adding an unlikely(), because the check is actually more of
an assertion.Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu")
Signed-off-by: Linus Lüssing
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The function batadv_frag_skb_buffer was supposed not to consume the skbuff
on errors. This was followed in the helper function
batadv_frag_insert_packet when the skb would potentially be inserted in the
fragment queue. But it could happen that the next helper function
batadv_frag_merge_packets would try to merge the fragments and fail. This
results in a kfree_skb of all the enqueued fragments (including the just
inserted one). batadv_recv_frag_packet would detect the error in
batadv_frag_skb_buffer and try to free the skb again.The behavior of batadv_frag_skb_buffer (and its helper
batadv_frag_insert_packet) must therefore be changed to always consume the
skbuff to have a common behavior and avoid the double kfree_skb.Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge")
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
30 Jan, 2017
1 commit
-
Simon Wunderlich says:
====================
Here are two fixes for batman-adv for net-next:- fix double call of dev_queue_xmit(), caused by the recent introduction
of net_xmit_eval(), by Sven Eckelmann- Fix includes for IS_ERR/ERR_PTR, by Sven Eckelmann
====================Signed-off-by: David S. Miller
28 Jan, 2017
3 commits
-
Two trivial overlapping changes conflicts in MPLS and mlx5.
Signed-off-by: David S. Miller
-
IS_ERR/ERR_PTR are not defined in linux/device.h but in linux/err.h. The
files using these macros therefore have to include the correct one.Reported-by: Linus Luessing
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
The net_xmit_eval has side effects because it is not making sure that e
isn't evaluated twice.#define net_xmit_eval(e) ((e) == NET_XMIT_CN ? 0 : (e))
The code requested by David Miller [1]
return net_xmit_eval(dev_queue_xmit(skb));
will get transformed into
return ((dev_queue_xmit(skb)) == NET_XMIT_CN ? 0 : (dev_queue_xmit(skb)))
dev_queue_xmit will therefore be tried again (with an already consumed skb)
whenever the return code is not NET_XMIT_CN.[1] https://lkml.kernel.org/r/20170125.225624.965229145391320056.davem@davemloft.net
Fixes: c33705188c49 ("batman-adv: Treat NET_XMIT_CN as transmit successfully")
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
27 Jan, 2017
1 commit
-
Simon Wunderlich says:
====================
This feature/cleanup patchset includes the following patches:- bump version strings, by Simon Wunderlich
- ignore self-generated loop detect MAC addresses in translation table,
by Simon Wunderlich- install uapi batman_adv.h header, by Sven Eckelmann
- bump copyright years, by Sven Eckelmann
- Remove an unused variable in translation table code, by Sven Eckelmann
- Handle NET_XMIT_CN like NET_XMIT_SUCCESS (revised according to Davids
suggestion), and a follow up code clean up, by Gao Feng (2 patches)
====================Signed-off-by: David S. Miller
26 Jan, 2017
3 commits
-
The tc could return NET_XMIT_CN as one congestion notification, but
it does not mean the packet is lost. Other modules like ipvlan,
macvlan, and others treat NET_XMIT_CN as success too.So batman-adv should handle NET_XMIT_CN also as NET_XMIT_SUCCESS.
Signed-off-by: Gao Feng
[sven@narfation.org: Moved NET_XMIT_CN handling to batadv_send_skb_packet]
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
It could decrease one condition check to collect some statements in the
first condition block.Signed-off-by: Gao Feng
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich -
Signed-off-by: Sven Eckelmann
Signed-off-by: Simon Wunderlich
