ACK/cmnt: [SRU][Zesty][PATCH 0/7] Fixes for LP#1715812

Stefan Bader stefan.bader at canonical.com
Fri Sep 15 09:19:13 UTC 2017


On 08.09.2017 09:00, Daniel Axtens wrote:
> (This is the Zesty patch-set. These are all clean cherry-picks.)
> 
> [SRU Justification]
> 
> [Impact]
> A host can lose access to another host whose MAC address changes if
> they have active connections to other hosts that share a route. The
> ARP cache does not time out as expected - instead the old MAC address
> is continuously reconfirmed.
> 
> [Fix]
> Apply series [1], which changes the algorithm for neighbour confirmation.
> That is, from upstream:
> 51ce8bd4d17a net: pending_confirm is not used anymore
> 0dec879f636f net: use dst_confirm_neigh for UDP, RAW, ICMP, L2TP
> 63fca65d0863 net: add confirm_neigh method to dst_ops
> c3a2e8370534 tcp: replace dst_confirm with sk_dst_confirm
> c86a773c7802 sctp: add dst_pending_confirm flag
> 4ff0620354f2 net: add dst_pending_confirm flag to skbuff
> 9b8805a32559 sock: add sk_dst_pending_confirm flag
> 
> [Test case]
> Create 3 real or virtual systems, all hooked up to a switch.
> One system needs an active-backup bond with fail_over_mac=1 num_grat_arp=0.
> 
> Put all the systems in the same subnet, e.g. 192.168.200.0/24
> 
> Call the system with the bond A, and the other two systems B and C.
> 
> On B, run in 3 shells:
>  - netperf -t TCP_RR to C
>  - ping -f A
>  - watch 'ip -s neigh show 192.168.200.0/24'
> 
> On A, cause the bond to fail over.
> 
> Observe that:
> 
>  - without the patches, B intermittently fails to notice the change in
>    A's MAC address. This presents as the ping failing and not
>    recovering, and the arp table showing the old mac address never
>    timing out and never being replace with a new mac address.
> 
>  - with the patches, the arp cache times out and B sends another mac
>    probe and detects A's new address.
> 
> It helps to use taskset to put ping and netperf on the same CPU, or
> use single-CPU vms.
> 
> See [2] for more details.
> 
> [References]
> [2] Original report: https://www.mail-archive.com/netdev@vger.kernel.org/msg138762.html
> [1]: https://www.spinics.net/lists/linux-rdma/msg45907.html
> 
> Julian Anastasov (7):
>   sock: add sk_dst_pending_confirm flag
>   net: add dst_pending_confirm flag to skbuff
>   sctp: add dst_pending_confirm flag
>   tcp: replace dst_confirm with sk_dst_confirm
>   net: add confirm_neigh method to dst_ops
>   net: use dst_confirm_neigh for UDP, RAW, ICMP, L2TP
>   net: pending_confirm is not used anymore
> 
>  drivers/net/vrf.c          |  5 ++++-
>  include/linux/skbuff.h     | 12 ++++++++++++
>  include/net/arp.h          | 16 ++++++++++++++++
>  include/net/dst.h          | 21 +++++++++------------
>  include/net/dst_ops.h      |  2 ++
>  include/net/ndisc.h        | 17 +++++++++++++++++
>  include/net/sctp/sctp.h    |  6 ++----
>  include/net/sctp/structs.h |  4 ++++
>  include/net/sock.h         | 26 ++++++++++++++++++++++++++
>  net/core/dst.c             |  1 -
>  net/core/sock.c            |  2 ++
>  net/ipv4/ip_output.c       | 11 ++++++++++-
>  net/ipv4/ping.c            |  3 ++-
>  net/ipv4/raw.c             |  6 +++++-
>  net/ipv4/route.c           | 19 +++++++++++++++++++
>  net/ipv4/tcp_input.c       | 12 +++---------
>  net/ipv4/tcp_metrics.c     |  7 ++-----
>  net/ipv4/tcp_output.c      |  2 ++
>  net/ipv4/udp.c             |  3 ++-
>  net/ipv6/ip6_output.c      |  7 +++++++
>  net/ipv6/raw.c             |  6 +++++-
>  net/ipv6/route.c           | 43 ++++++++++++++++++++++++++++++-------------
>  net/ipv6/udp.c             |  3 ++-
>  net/l2tp/l2tp_ip6.c        |  3 ++-
>  net/sctp/associola.c       |  3 +--
>  net/sctp/output.c          | 10 +++++++++-
>  net/sctp/outqueue.c        |  2 +-
>  net/sctp/sm_make_chunk.c   |  6 ++----
>  net/sctp/sm_sideeffect.c   |  2 +-
>  net/sctp/socket.c          |  4 ++--
>  net/sctp/transport.c       | 16 +++++++++++++++-
>  net/xfrm/xfrm_policy.c     | 19 +++++++++++++++++++
>  32 files changed, 235 insertions(+), 64 deletions(-)
> 
Acked-by: Stefan Bader <stefan.bader at canonical.com>

As Colin, I had my reservations on this set because it changes a relatively
large number of files all over the place. But as it was successfully tested
(both for verification and against potential regressions) and has no fixup since
4.11 (when the last patch of the set was included), plus some explanations by
Jay, I guess this is of acceptable risk.

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20170915/416f2650/attachment.sig>


More information about the kernel-team mailing list