• Sebastian Andrzej Siewior's avatar
    hsr: Synchronize sending frames to have always incremented outgoing seq nr. · 06afd2c3
    Sebastian Andrzej Siewior authored
    Sending frames via the hsr (master) device requires a sequence number
    which is tracked in hsr_priv::sequence_nr and protected by
    hsr_priv::seqnr_lock. Each time a new frame is sent, it will obtain a
    new id and then send it via the slave devices.
    Each time a packet is sent (via hsr_forward_do()) the sequence number is
    checked via hsr_register_frame_out() to ensure that a frame is not
    handled twice. This make sense for the receiving side to ensure that the
    frame is not injected into the stack twice after it has been received
    from both slave ports.
    
    There is no locking to cover the sending path which means the following
    scenario is possible:
    
      CPU0				CPU1
      hsr_dev_xmit(skb1)		hsr_dev_xmit(skb2)
       fill_frame_info()             fill_frame_info()
        hsr_fill_frame_info()         hsr_fill_frame_info()
         handle_std_frame()            handle_std_frame()
          skb1's sequence_nr = 1
                                        skb2's sequence_nr = 2
       hsr_forward_do()              hsr_forward_do()
    
                                       hsr_register_frame_out(, 2)  // okay, send)
    
        hsr_register_frame_out(, 1) // stop, lower seq duplicate
    
    Both skbs (or their struct hsr_frame_info) received an unique id.
    However since skb2 was sent before skb1, the higher sequence number was
    recorded in hsr_register_frame_out() and the late arriving skb1 was
    dropped and never sent.
    
    This scenario has been observed in a three node HSR setup, with node1 +
    node2 having ping and iperf running in parallel. From time to time ping
    reported a missing packet. Based on tracing that missing ping packet did
    not leave the system.
    
    It might be possible (didn't check) to drop the sequence number check on
    the sending side. But if the higher sequence number leaves on wire
    before the lower does and the destination receives them in that order
    and it will drop the packet with the lower sequence number and never
    inject into the stack.
    Therefore it seems the only way is to lock the whole path from obtaining
    the sequence number and sending via dev_queue_xmit() and assuming the
    packets leave on wire in the same order (and don't get reordered by the
    NIC).
    
    Cover the whole path for the master interface from obtaining the ID
    until after it has been forwarded via hsr_forward_skb() to ensure the
    skbs are sent to the NIC in the order of the assigned sequence numbers.
    
    Fixes: f421436a ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
    Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    06afd2c3
hsr_forward.c 16.8 KB