Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

rxrpc: Fix counting of new acks and nacks

Fix the counting of new acks and nacks when parsing a packet - something
that is used in congestion control.

As the code stands, it merely notes if there are any nacks whereas what we
really should do is compare the previous SACK table to the new one,
assuming we get two successive ACK packets with nacks in them. However, we
really don't want to do that if we can avoid it as the tables might not
correspond directly as one may be shifted from the other - something that
will only get harder to deal with once extended ACK tables come into full
use (with a capacity of up to 8192).

Instead, count the number of nacks shifted out of the old SACK, the number
of nacks retained in the portion still active and the number of new acks
and nacks in the new table then calculate what we need.

Note this ends up a bit of an estimate as the Rx protocol allows acks to be
withdrawn by the receiver and packets requested to be retransmitted.

Fixes: d57a3a151660 ("rxrpc: Save last ACK's SACK table rather than marking txbufs")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

David Howells and committed by
David S. Miller
41b7fa15 6f769f22

+122 -28
+5 -3
include/trace/events/rxrpc.h
··· 128 128 EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \ 129 129 EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \ 130 130 EM(rxrpc_skb_get_conn_work, "GET conn-work") \ 131 + EM(rxrpc_skb_get_last_nack, "GET last-nack") \ 131 132 EM(rxrpc_skb_get_local_work, "GET locl-work") \ 132 133 EM(rxrpc_skb_get_reject_work, "GET rej-work ") \ 133 134 EM(rxrpc_skb_get_to_recvmsg, "GET to-recv ") \ ··· 142 141 EM(rxrpc_skb_put_error_report, "PUT error-rep") \ 143 142 EM(rxrpc_skb_put_input, "PUT input ") \ 144 143 EM(rxrpc_skb_put_jumbo_subpacket, "PUT jumbo-sub") \ 144 + EM(rxrpc_skb_put_last_nack, "PUT last-nack") \ 145 145 EM(rxrpc_skb_put_purge, "PUT purge ") \ 146 146 EM(rxrpc_skb_put_rotate, "PUT rotate ") \ 147 147 EM(rxrpc_skb_put_unknown, "PUT unknown ") \ ··· 1554 1552 memcpy(&__entry->sum, summary, sizeof(__entry->sum)); 1555 1553 ), 1556 1554 1557 - TP_printk("c=%08x r=%08x %s q=%08x %s cw=%u ss=%u nA=%u,%u+%u r=%u b=%u u=%u d=%u l=%x%s%s%s", 1555 + TP_printk("c=%08x r=%08x %s q=%08x %s cw=%u ss=%u nA=%u,%u+%u,%u b=%u u=%u d=%u l=%x%s%s%s", 1558 1556 __entry->call, 1559 1557 __entry->ack_serial, 1560 1558 __print_symbolic(__entry->sum.ack_reason, rxrpc_ack_names), ··· 1562 1560 __print_symbolic(__entry->sum.mode, rxrpc_congest_modes), 1563 1561 __entry->sum.cwnd, 1564 1562 __entry->sum.ssthresh, 1565 - __entry->sum.nr_acks, __entry->sum.saw_nacks, 1563 + __entry->sum.nr_acks, __entry->sum.nr_retained_nacks, 1566 1564 __entry->sum.nr_new_acks, 1567 - __entry->sum.nr_rot_new_acks, 1565 + __entry->sum.nr_new_nacks, 1568 1566 __entry->top - __entry->hard_ack, 1569 1567 __entry->sum.cumulative_acks, 1570 1568 __entry->sum.dup_acks,
+15 -5
net/rxrpc/ar-internal.h
··· 199 199 */ 200 200 struct rxrpc_skb_priv { 201 201 struct rxrpc_connection *conn; /* Connection referred to (poke packet) */ 202 - u16 offset; /* Offset of data */ 203 - u16 len; /* Length of data */ 204 - u8 flags; 202 + union { 203 + struct { 204 + u16 offset; /* Offset of data */ 205 + u16 len; /* Length of data */ 206 + u8 flags; 205 207 #define RXRPC_RX_VERIFIED 0x01 206 - 208 + }; 209 + struct { 210 + rxrpc_seq_t first_ack; /* First packet in acks table */ 211 + u8 nr_acks; /* Number of acks+nacks */ 212 + u8 nr_nacks; /* Number of nacks */ 213 + }; 214 + }; 207 215 struct rxrpc_host_header hdr; /* RxRPC packet header from this packet */ 208 216 }; 209 217 ··· 700 692 u8 cong_dup_acks; /* Count of ACKs showing missing packets */ 701 693 u8 cong_cumul_acks; /* Cumulative ACK count */ 702 694 ktime_t cong_tstamp; /* Last time cwnd was changed */ 695 + struct sk_buff *cong_last_nack; /* Last ACK with nacks received */ 703 696 704 697 /* Receive-phase ACK management (ACKs we send). */ 705 698 u8 ackr_reason; /* reason to ACK */ ··· 738 729 struct rxrpc_ack_summary { 739 730 u16 nr_acks; /* Number of ACKs in packet */ 740 731 u16 nr_new_acks; /* Number of new ACKs in packet */ 741 - u16 nr_rot_new_acks; /* Number of rotated new ACKs */ 732 + u16 nr_new_nacks; /* Number of new nacks in packet */ 733 + u16 nr_retained_nacks; /* Number of nacks retained between ACKs */ 742 734 u8 ack_reason; 743 735 bool saw_nacks; /* Saw NACKs in packet */ 744 736 bool new_low_nack; /* T if new low NACK found */
+4 -2
net/rxrpc/call_event.c
··· 112 112 void rxrpc_resend(struct rxrpc_call *call, struct sk_buff *ack_skb) 113 113 { 114 114 struct rxrpc_ackpacket *ack = NULL; 115 + struct rxrpc_skb_priv *sp; 115 116 struct rxrpc_txbuf *txb; 116 117 unsigned long resend_at; 117 118 rxrpc_seq_t transmitted = READ_ONCE(call->tx_transmitted); ··· 140 139 * explicitly NAK'd packets. 141 140 */ 142 141 if (ack_skb) { 142 + sp = rxrpc_skb(ack_skb); 143 143 ack = (void *)ack_skb->data + sizeof(struct rxrpc_wire_header); 144 144 145 - for (i = 0; i < ack->nAcks; i++) { 145 + for (i = 0; i < sp->nr_acks; i++) { 146 146 rxrpc_seq_t seq; 147 147 148 148 if (ack->acks[i] & 1) 149 149 continue; 150 - seq = ntohl(ack->firstPacket) + i; 150 + seq = sp->first_ack + i; 151 151 if (after(txb->seq, transmitted)) 152 152 break; 153 153 if (after(txb->seq, seq))
+1
net/rxrpc/call_object.c
··· 686 686 687 687 del_timer_sync(&call->timer); 688 688 689 + rxrpc_free_skb(call->cong_last_nack, rxrpc_skb_put_last_nack); 689 690 rxrpc_cleanup_ring(call); 690 691 while ((txb = list_first_entry_or_null(&call->tx_sendmsg, 691 692 struct rxrpc_txbuf, call_link))) {
+97 -18
net/rxrpc/input.c
··· 45 45 } 46 46 47 47 cumulative_acks += summary->nr_new_acks; 48 - cumulative_acks += summary->nr_rot_new_acks; 49 48 if (cumulative_acks > 255) 50 49 cumulative_acks = 255; 51 50 52 - summary->mode = call->cong_mode; 53 51 summary->cwnd = call->cong_cwnd; 54 52 summary->ssthresh = call->cong_ssthresh; 55 53 summary->cumulative_acks = cumulative_acks; ··· 149 151 cwnd = RXRPC_TX_MAX_WINDOW; 150 152 call->cong_cwnd = cwnd; 151 153 call->cong_cumul_acks = cumulative_acks; 154 + summary->mode = call->cong_mode; 152 155 trace_rxrpc_congest(call, summary, acked_serial, change); 153 156 if (resend) 154 157 rxrpc_resend(call, skb); ··· 212 213 list_for_each_entry_rcu(txb, &call->tx_buffer, call_link, false) { 213 214 if (before_eq(txb->seq, call->acks_hard_ack)) 214 215 continue; 215 - summary->nr_rot_new_acks++; 216 216 if (test_bit(RXRPC_TXBUF_LAST, &txb->flags)) { 217 217 set_bit(RXRPC_CALL_TX_LAST, &call->flags); 218 218 rot_last = true; ··· 251 253 enum rxrpc_abort_reason abort_why) 252 254 { 253 255 ASSERT(test_bit(RXRPC_CALL_TX_LAST, &call->flags)); 256 + 257 + if (unlikely(call->cong_last_nack)) { 258 + rxrpc_free_skb(call->cong_last_nack, rxrpc_skb_put_last_nack); 259 + call->cong_last_nack = NULL; 260 + } 254 261 255 262 switch (__rxrpc_call_state(call)) { 256 263 case RXRPC_CALL_CLIENT_SEND_REQUEST: ··· 706 703 } 707 704 708 705 /* 706 + * Determine how many nacks from the previous ACK have now been satisfied. 707 + */ 708 + static rxrpc_seq_t rxrpc_input_check_prev_ack(struct rxrpc_call *call, 709 + struct rxrpc_ack_summary *summary, 710 + rxrpc_seq_t seq) 711 + { 712 + struct sk_buff *skb = call->cong_last_nack; 713 + struct rxrpc_ackpacket ack; 714 + struct rxrpc_skb_priv *sp = rxrpc_skb(skb); 715 + unsigned int i, new_acks = 0, retained_nacks = 0; 716 + rxrpc_seq_t old_seq = sp->first_ack; 717 + u8 *acks = skb->data + sizeof(struct rxrpc_wire_header) + sizeof(ack); 718 + 719 + if (after_eq(seq, old_seq + sp->nr_acks)) { 720 + summary->nr_new_acks += sp->nr_nacks; 721 + summary->nr_new_acks += seq - (old_seq + sp->nr_acks); 722 + summary->nr_retained_nacks = 0; 723 + } else if (seq == old_seq) { 724 + summary->nr_retained_nacks = sp->nr_nacks; 725 + } else { 726 + for (i = 0; i < sp->nr_acks; i++) { 727 + if (acks[i] == RXRPC_ACK_TYPE_NACK) { 728 + if (before(old_seq + i, seq)) 729 + new_acks++; 730 + else 731 + retained_nacks++; 732 + } 733 + } 734 + 735 + summary->nr_new_acks += new_acks; 736 + summary->nr_retained_nacks = retained_nacks; 737 + } 738 + 739 + return old_seq + sp->nr_acks; 740 + } 741 + 742 + /* 709 743 * Process individual soft ACKs. 710 744 * 711 745 * Each ACK in the array corresponds to one packet and can be either an ACK or ··· 751 711 * the timer on the basis that the peer might just not have processed them at 752 712 * the time the ACK was sent. 753 713 */ 754 - static void rxrpc_input_soft_acks(struct rxrpc_call *call, u8 *acks, 755 - rxrpc_seq_t seq, int nr_acks, 756 - struct rxrpc_ack_summary *summary) 714 + static void rxrpc_input_soft_acks(struct rxrpc_call *call, 715 + struct rxrpc_ack_summary *summary, 716 + struct sk_buff *skb, 717 + rxrpc_seq_t seq, 718 + rxrpc_seq_t since) 757 719 { 758 - unsigned int i; 720 + struct rxrpc_skb_priv *sp = rxrpc_skb(skb); 721 + unsigned int i, old_nacks = 0; 722 + rxrpc_seq_t lowest_nak = seq + sp->nr_acks; 723 + u8 *acks = skb->data + sizeof(struct rxrpc_wire_header) + sizeof(struct rxrpc_ackpacket); 759 724 760 - for (i = 0; i < nr_acks; i++) { 725 + for (i = 0; i < sp->nr_acks; i++) { 761 726 if (acks[i] == RXRPC_ACK_TYPE_ACK) { 762 727 summary->nr_acks++; 763 - summary->nr_new_acks++; 728 + if (after_eq(seq, since)) 729 + summary->nr_new_acks++; 764 730 } else { 765 - if (!summary->saw_nacks && 766 - call->acks_lowest_nak != seq + i) { 767 - call->acks_lowest_nak = seq + i; 768 - summary->new_low_nack = true; 769 - } 770 731 summary->saw_nacks = true; 732 + if (before(seq, since)) { 733 + /* Overlap with previous ACK */ 734 + old_nacks++; 735 + } else { 736 + summary->nr_new_nacks++; 737 + sp->nr_nacks++; 738 + } 739 + 740 + if (before(seq, lowest_nak)) 741 + lowest_nak = seq; 771 742 } 743 + seq++; 772 744 } 745 + 746 + if (lowest_nak != call->acks_lowest_nak) { 747 + call->acks_lowest_nak = lowest_nak; 748 + summary->new_low_nack = true; 749 + } 750 + 751 + /* We *can* have more nacks than we did - the peer is permitted to drop 752 + * packets it has soft-acked and re-request them. Further, it is 753 + * possible for the nack distribution to change whilst the number of 754 + * nacks stays the same or goes down. 755 + */ 756 + if (old_nacks < summary->nr_retained_nacks) 757 + summary->nr_new_acks += summary->nr_retained_nacks - old_nacks; 758 + summary->nr_retained_nacks = old_nacks; 773 759 } 774 760 775 761 /* ··· 839 773 struct rxrpc_skb_priv *sp = rxrpc_skb(skb); 840 774 struct rxrpc_ackinfo info; 841 775 rxrpc_serial_t ack_serial, acked_serial; 842 - rxrpc_seq_t first_soft_ack, hard_ack, prev_pkt; 776 + rxrpc_seq_t first_soft_ack, hard_ack, prev_pkt, since; 843 777 int nr_acks, offset, ioffset; 844 778 845 779 _enter(""); ··· 855 789 prev_pkt = ntohl(ack.previousPacket); 856 790 hard_ack = first_soft_ack - 1; 857 791 nr_acks = ack.nAcks; 792 + sp->first_ack = first_soft_ack; 793 + sp->nr_acks = nr_acks; 858 794 summary.ack_reason = (ack.reason < RXRPC_ACK__INVALID ? 859 795 ack.reason : RXRPC_ACK__INVALID); 860 796 ··· 926 858 if (nr_acks > 0) 927 859 skb_condense(skb); 928 860 861 + if (call->cong_last_nack) { 862 + since = rxrpc_input_check_prev_ack(call, &summary, first_soft_ack); 863 + rxrpc_free_skb(call->cong_last_nack, rxrpc_skb_put_last_nack); 864 + call->cong_last_nack = NULL; 865 + } else { 866 + summary.nr_new_acks = first_soft_ack - call->acks_first_seq; 867 + call->acks_lowest_nak = first_soft_ack + nr_acks; 868 + since = first_soft_ack; 869 + } 870 + 929 871 call->acks_latest_ts = skb->tstamp; 930 872 call->acks_first_seq = first_soft_ack; 931 873 call->acks_prev_seq = prev_pkt; ··· 944 866 case RXRPC_ACK_PING: 945 867 break; 946 868 default: 947 - if (after(acked_serial, call->acks_highest_serial)) 869 + if (acked_serial && after(acked_serial, call->acks_highest_serial)) 948 870 call->acks_highest_serial = acked_serial; 949 871 break; 950 872 } ··· 983 905 if (nr_acks > 0) { 984 906 if (offset > (int)skb->len - nr_acks) 985 907 return rxrpc_proto_abort(call, 0, rxrpc_eproto_ackr_short_sack); 986 - rxrpc_input_soft_acks(call, skb->data + offset, first_soft_ack, 987 - nr_acks, &summary); 908 + rxrpc_input_soft_acks(call, &summary, skb, first_soft_ack, since); 909 + rxrpc_get_skb(skb, rxrpc_skb_get_last_nack); 910 + call->cong_last_nack = skb; 988 911 } 989 912 990 913 if (test_bit(RXRPC_CALL_TX_LAST, &call->flags) &&