Re: git: c620788150d2 - main - sctp: keep sb_acc and sb_ccc in sync

On Fri, Jul 28, 2023 at 10:17:28PM +0000, Michael Tuexen wrote:
> The branch main has been updated by tuexen:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=c620788150d274c09a070ab486602c98407d73b0
> 
> commit c620788150d274c09a070ab486602c98407d73b0
> Author:     Michael Tuexen <tuexen@FreeBSD.org>
> AuthorDate: 2023-07-28 13:16:23 +0000
> Commit:     Michael Tuexen <tuexen@FreeBSD.org>
> CommitDate: 2023-07-28 13:16:23 +0000
> 
>     sctp: keep sb_acc and sb_ccc in sync
>     
>     PR:             260116
>     MFC after:      1 week
> ---

Hi, Michael,
perhaps some of the recent fixes have broken these tests:
https://ci.freebsd.org/view/all/job/FreeBSD-main-amd64-test/lastFailedBuild/console

02:04:05 sys/netpfil/pf/sctp:basic_v4  ->  epair1a: Ethernet address: 02:b9:89:4e:84:0a
02:04:05 epair1b: Ethernet address: 02:b9:89:4e:84:0b
02:04:05 epair1a: link state changed to UP
02:04:05 epair1b: link state changed to UP
02:04:06 panic: Counter goes negative
02:04:06 cpuid = 1
02:04:06 time = 1690758247
02:04:06 KDB: stack backtrace:
02:04:06 db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe00a0775480
02:04:06 vpanic() at vpanic+0x149/frame 0xfffffe00a07754d0
02:04:06 panic() at panic+0x43/frame 0xfffffe00a0775530
02:04:06 sctp_abort_notification() at sctp_abort_notification/frame 0xfffffe00a0775540
02:04:06 sctp_express_handle_sack() at sctp_express_handle_sack+0x647/frame 0xfffffe00a0775640
02:04:06 sctp_process_control() at sctp_process_control+0xf62/frame 0xfffffe00a0775990
02:04:06 sctp_common_input_processing() at sctp_common_input_processing+0x561/frame 0xfffffe00a0775b10
02:04:06 sctp_input_with_port() at sctp_input_with_port+0x1fa/frame 0xfffffe00a0775be0
02:04:06 sctp_input() at sctp_input+0x10/frame 0xfffffe00a0775bf0
02:04:06 ip_input() at ip_input+0x2ab/frame 0xfffffe00a0775c50
02:04:06 netisr_dispatch_src() at netisr_dispatch_src+0xad/frame 0xfffffe00a0775cb0
02:04:06 ether_demux() at ether_demux+0x17a/frame 0xfffffe00a0775ce0
02:04:06 ether_nh_input() at ether_nh_input+0x39f/frame 0xfffffe00a0775d30
02:04:06 netisr_dispatch_src() at netisr_dispatch_src+0xad/frame 0xfffffe00a0775d90
02:04:06 ether_input() at ether_input+0xd9/frame 0xfffffe00a0775df0
02:04:06 epair_tx_start_deferred() at epair_tx_start_deferred+0xd7/frame 0xfffffe00a0775e40
02:04:06 taskqueue_run_locked() at taskqueue_run_locked+0xab/frame 0xfffffe00a0775ec0
02:04:06 taskqueue_thread_loop() at taskqueue_thread_loop+0xd3/frame 0xfffffe00a0775ef0
02:04:06 fork_exit() at fork_exit+0x82/frame 0xfffffe00a0775f30
02:04:06 fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe00a0775f30
02:04:06 --- trap 0xc, rip = 0xf04bf926c9a, rsp = 0xf04c04a4f48, rbp = 0xf04c04a4f60 ---


>  sys/netinet/sctp_os_bsd.h | 18 ++++++++++++------
>  sys/netinet/sctp_output.c |  2 +-
>  sys/netinet/sctp_pcb.c    |  2 +-
>  sys/netinet/sctp_var.h    |  4 ++--
>  sys/netinet/sctputil.c    |  4 ++--
>  sys/netinet/sctputil.h    |  4 ++--
>  6 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/sys/netinet/sctp_os_bsd.h b/sys/netinet/sctp_os_bsd.h
> index 80c187b3c5ef..0d88377523c8 100644
> --- a/sys/netinet/sctp_os_bsd.h
> +++ b/sys/netinet/sctp_os_bsd.h
> @@ -372,11 +372,6 @@ typedef struct callout sctp_os_timer_t;
>  #define SCTP_CLEAR_SO_NBIO(so)	((so)->so_state &= ~SS_NBIO)
>  /* get the socket type */
>  #define SCTP_SO_TYPE(so)	((so)->so_type)
> -/* Use a macro for renaming sb_cc to sb_acc.
> - * Initially sb_ccc was used, but this broke select() when used
> - * with SCTP sockets.
> - */
> -#define sb_cc sb_acc
>  /* reserve sb space for a socket */
>  #define SCTP_SORESERVE(so, send, recv)	soreserve(so, send, recv)
>  /* wakeup a socket */
> @@ -384,8 +379,19 @@ typedef struct callout sctp_os_timer_t;
>  /* number of bytes ready to read */
>  #define SCTP_SBAVAIL(sb)	sbavail(sb)
>  /* clear the socket buffer state */
> +#define SCTP_SB_INCR(sb, incr)			\
> +{						\
> +	atomic_add_int(&(sb)->sb_acc, incr);	\
> +	atomic_add_int(&(sb)->sb_ccc, incr);	\
> +}
> +#define SCTP_SB_DECR(sb, decr)					\
> +{								\
> +	SCTP_SAVE_ATOMIC_DECREMENT(&(sb)->sb_acc, decr);	\
> +	SCTP_SAVE_ATOMIC_DECREMENT(&(sb)->sb_ccc, decr);	\
> +}
>  #define SCTP_SB_CLEAR(sb)	\
> -	(sb).sb_cc = 0;		\
> +	(sb).sb_acc = 0;	\
> +	(sb).sb_ccc = 0;	\
>  	(sb).sb_mb = NULL;	\
>  	(sb).sb_mbcnt = 0;
>  
> diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c
> index a146ad401b30..7e32cc662f38 100644
> --- a/sys/netinet/sctp_output.c
> +++ b/sys/netinet/sctp_output.c
> @@ -7290,7 +7290,7 @@ one_more_time:
>  			if ((stcb->sctp_socket != NULL) &&
>  			    ((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) ||
>  			    (stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL))) {
> -				atomic_subtract_int(&stcb->sctp_socket->so_snd.sb_cc, sp->length);
> +				SCTP_SB_DECR(&stcb->sctp_socket->so_snd, sp->length);
>  			}
>  			if (sp->data) {
>  				sctp_m_freem(sp->data);
> diff --git a/sys/netinet/sctp_pcb.c b/sys/netinet/sctp_pcb.c
> index 9df2f82bdaa6..220d8b72667d 100644
> --- a/sys/netinet/sctp_pcb.c
> +++ b/sys/netinet/sctp_pcb.c
> @@ -3612,7 +3612,7 @@ sctp_inpcb_free(struct sctp_inpcb *inp, int immediate, int from)
>  		TAILQ_REMOVE(&inp->read_queue, sq, next);
>  		sctp_free_remote_addr(sq->whoFrom);
>  		if (so)
> -			so->so_rcv.sb_cc -= sq->length;
> +			SCTP_SB_DECR(&so->so_rcv, sq->length);
>  		if (sq->data) {
>  			sctp_m_freem(sq->data);
>  			sq->data = NULL;
> diff --git a/sys/netinet/sctp_var.h b/sys/netinet/sctp_var.h
> index 8301ec38cf6e..ef01b0b6c3bd 100644
> --- a/sys/netinet/sctp_var.h
> +++ b/sys/netinet/sctp_var.h
> @@ -198,7 +198,7 @@ extern struct protosw sctp_seqpacket_protosw, sctp_stream_protosw;
>  }
>  
>  #define sctp_sbfree(ctl, stcb, sb, m) { \
> -	SCTP_SAVE_ATOMIC_DECREMENT(&(sb)->sb_cc, SCTP_BUF_LEN((m))); \
> +	SCTP_SB_DECR(sb, SCTP_BUF_LEN((m))); \
>  	SCTP_SAVE_ATOMIC_DECREMENT(&(sb)->sb_mbcnt, MSIZE); \
>  	if (((ctl)->do_not_ref_stcb == 0) && stcb) {\
>  		SCTP_SAVE_ATOMIC_DECREMENT(&(stcb)->asoc.sb_cc, SCTP_BUF_LEN((m))); \
> @@ -210,7 +210,7 @@ extern struct protosw sctp_seqpacket_protosw, sctp_stream_protosw;
>  }
>  
>  #define sctp_sballoc(stcb, sb, m) { \
> -	atomic_add_int(&(sb)->sb_cc,SCTP_BUF_LEN((m))); \
> +	SCTP_SB_INCR(sb, SCTP_BUF_LEN((m))); \
>  	atomic_add_int(&(sb)->sb_mbcnt, MSIZE); \
>  	if (stcb) { \
>  		atomic_add_int(&(stcb)->asoc.sb_cc, SCTP_BUF_LEN((m))); \
> diff --git a/sys/netinet/sctputil.c b/sys/netinet/sctputil.c
> index ef8d763efabf..a4d4e806c00e 100644
> --- a/sys/netinet/sctputil.c
> +++ b/sys/netinet/sctputil.c
> @@ -5044,7 +5044,7 @@ sctp_free_bufspace(struct sctp_tcb *stcb, struct sctp_association *asoc,
>  	if ((stcb->sctp_socket != NULL) &&
>  	    (((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL)) ||
>  	    ((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE)))) {
> -		SCTP_SAVE_ATOMIC_DECREMENT(&stcb->sctp_socket->so_snd.sb_cc, tp1->book_size);
> +		SCTP_SB_DECR(&stcb->sctp_socket->so_snd, tp1->book_size);
>  	}
>  }
>  
> @@ -6121,7 +6121,7 @@ get_more_data:
>  					if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_SB_LOGGING_ENABLE) {
>  						sctp_sblog(&so->so_rcv, control->do_not_ref_stcb ? NULL : stcb, SCTP_LOG_SBFREE, (int)cp_len);
>  					}
> -					SCTP_SAVE_ATOMIC_DECREMENT(&so->so_rcv.sb_cc, (int)cp_len);
> +					SCTP_SB_DECR(&so->so_rcv, (int)cp_len);
>  					if ((control->do_not_ref_stcb == 0) &&
>  					    stcb) {
>  						atomic_subtract_int(&stcb->asoc.sb_cc, (int)cp_len);
> diff --git a/sys/netinet/sctputil.h b/sys/netinet/sctputil.h
> index 60b5661c309c..d74780cdccf7 100644
> --- a/sys/netinet/sctputil.h
> +++ b/sys/netinet/sctputil.h
> @@ -253,7 +253,7 @@ do { \
>  		} \
>  		if (stcb->sctp_socket && ((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) || \
>  		    (stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL))) { \
> -			SCTP_SAVE_ATOMIC_DECREMENT(&stcb->sctp_socket->so_snd.sb_cc, sp->length); \
> +			SCTP_SB_DECR(&stcb->sctp_socket->so_snd, sp->length); \
>  		} \
>  	} \
>  } while (0)
> @@ -264,7 +264,7 @@ do { \
>  	if ((stcb->sctp_socket != NULL) && \
>  	    ((stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) || \
>  	     (stcb->sctp_ep->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL))) { \
> -		atomic_add_int(&stcb->sctp_socket->so_snd.sb_cc,sz); \
> +		SCTP_SB_INCR(&stcb->sctp_socket->so_snd, sz); \
>  	} \
>  } while (0)
>