diff mbox series

[net-next,06/14] sctp: do the basic send and recv for PLPMTUD probe

Message ID 66a73fb28cc8175ac80735f6301110b952f6e139.1624239422.git.lucien.xin@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series sctp: implement RFC8899: Packetization Layer Path MTU Discovery for SCTP transport | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: vyasevich@gmail.com nhorman@tuxdriver.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 59 this patch: 62
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns
netdev/build_allmodconfig_warn fail Errors and warnings before: 58 this patch: 61
netdev/header_inline success Link

Commit Message

Xin Long June 21, 2021, 1:38 a.m. UTC
This patch does exactly what rfc8899#section-6.2.1.2 says:

   The SCTP sender needs to be able to determine the total size of a
   probe packet.  The HEARTBEAT chunk could carry a Heartbeat
   Information parameter that includes, besides the information
   suggested in [RFC4960], the probe size to help an implementation
   associate a HEARTBEAT ACK with the size of probe that was sent.  The
   sender could also use other methods, such as sending a nonce and
   verifying the information returned also contains the corresponding
   nonce.  The length of the PAD chunk is computed by reducing the
   probing size by the size of the SCTP common header and the HEARTBEAT
   chunk.

Note that HB ACK chunk will carry back whatever HB chunk carried, including
the probe_size we put it in; We also check hbinfo->probe_size in the HB ACK
against link->pl.probe_size to validate this HB ACK chunk.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/sm.h      |  3 ++-
 include/net/sctp/structs.h |  2 ++
 net/sctp/output.c          | 33 ++++++++++++++++++++++++++++++++-
 net/sctp/outqueue.c        | 13 +++++++++++--
 net/sctp/sm_make_chunk.c   |  5 ++++-
 net/sctp/sm_statefuns.c    | 20 ++++++++++++++++++--
 6 files changed, 69 insertions(+), 7 deletions(-)

Comments

kernel test robot June 21, 2021, 3:49 a.m. UTC | #1
Hi Xin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/sctp-implement-RFC8899-Packetization-Layer-Path-MTU-Discovery-for-SCTP-transport/20210621-094007
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git adc2e56ebe6377f5c032d96aee0feac30a640453
config: i386-randconfig-r023-20210620 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/fcac1d6488c8bc7cb69af9e8051686a674d94fc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Xin-Long/sctp-implement-RFC8899-Packetization-Layer-Path-MTU-Discovery-for-SCTP-transport/20210621-094007
        git checkout fcac1d6488c8bc7cb69af9e8051686a674d94fc3
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/sctp/output.c:215:16: warning: no previous prototype for 'sctp_packet_bundle_pad' [-Wmissing-prototypes]
     215 | enum sctp_xmit sctp_packet_bundle_pad(struct sctp_packet *pkt, struct sctp_chunk *chunk)
         |                ^~~~~~~~~~~~~~~~~~~~~~
   net/sctp/output.c: In function 'sctp_packet_bundle_pad':
>> net/sctp/output.c:219:20: warning: variable 'sp' set but not used [-Wunused-but-set-variable]
     219 |  struct sctp_sock *sp;
         |                    ^~


vim +/sctp_packet_bundle_pad +215 net/sctp/output.c

   213	
   214	/* Try to bundle a pad chunk into a packet with a heartbeat chunk for PLPMTUTD probe */
 > 215	enum sctp_xmit sctp_packet_bundle_pad(struct sctp_packet *pkt, struct sctp_chunk *chunk)
   216	{
   217		struct sctp_transport *t = pkt->transport;
   218		struct sctp_chunk *pad;
 > 219		struct sctp_sock *sp;
   220		int overhead = 0;
   221	
   222		if (!chunk->pmtu_probe)
   223			return SCTP_XMIT_OK;
   224	
   225		sp = sctp_sk(t->asoc->base.sk);
   226	
   227		/* calculate the Padding Data size for the pad chunk */
   228		overhead += sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr);
   229		overhead += sizeof(struct sctp_sender_hb_info) + sizeof(struct sctp_pad_chunk);
   230		pad = sctp_make_pad(t->asoc, t->pl.probe_size - overhead);
   231		if (!pad)
   232			return SCTP_XMIT_DELAY;
   233	
   234		list_add_tail(&pad->list, &pkt->chunk_list);
   235		pkt->size += SCTP_PAD4(ntohs(pad->chunk_hdr->length));
   236		chunk->transport = t;
   237	
   238		return SCTP_XMIT_OK;
   239	}
   240	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Xin Long June 22, 2021, 1:13 a.m. UTC | #2
This is a "set but not used" warning, I can fix it after.

Thanks.

On Sun, Jun 20, 2021 at 11:50 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Xin,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
>
> url:    https://github.com/0day-ci/linux/commits/Xin-Long/sctp-implement-RFC8899-Packetization-Layer-Path-MTU-Discovery-for-SCTP-transport/20210621-094007
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git adc2e56ebe6377f5c032d96aee0feac30a640453
> config: i386-randconfig-r023-20210620 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/0day-ci/linux/commit/fcac1d6488c8bc7cb69af9e8051686a674d94fc3
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Xin-Long/sctp-implement-RFC8899-Packetization-Layer-Path-MTU-Discovery-for-SCTP-transport/20210621-094007
>         git checkout fcac1d6488c8bc7cb69af9e8051686a674d94fc3
>         # save the attached .config to linux build tree
>         make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> net/sctp/output.c:215:16: warning: no previous prototype for 'sctp_packet_bundle_pad' [-Wmissing-prototypes]
>      215 | enum sctp_xmit sctp_packet_bundle_pad(struct sctp_packet *pkt, struct sctp_chunk *chunk)
>          |                ^~~~~~~~~~~~~~~~~~~~~~
>    net/sctp/output.c: In function 'sctp_packet_bundle_pad':
> >> net/sctp/output.c:219:20: warning: variable 'sp' set but not used [-Wunused-but-set-variable]
>      219 |  struct sctp_sock *sp;
>          |                    ^~
>
>
> vim +/sctp_packet_bundle_pad +215 net/sctp/output.c
>
>    213
>    214  /* Try to bundle a pad chunk into a packet with a heartbeat chunk for PLPMTUTD probe */
>  > 215  enum sctp_xmit sctp_packet_bundle_pad(struct sctp_packet *pkt, struct sctp_chunk *chunk)
>    216  {
>    217          struct sctp_transport *t = pkt->transport;
>    218          struct sctp_chunk *pad;
>  > 219          struct sctp_sock *sp;
>    220          int overhead = 0;
>    221
>    222          if (!chunk->pmtu_probe)
>    223                  return SCTP_XMIT_OK;
>    224
>    225          sp = sctp_sk(t->asoc->base.sk);
>    226
>    227          /* calculate the Padding Data size for the pad chunk */
>    228          overhead += sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr);
>    229          overhead += sizeof(struct sctp_sender_hb_info) + sizeof(struct sctp_pad_chunk);
>    230          pad = sctp_make_pad(t->asoc, t->pl.probe_size - overhead);
>    231          if (!pad)
>    232                  return SCTP_XMIT_DELAY;
>    233
>    234          list_add_tail(&pad->list, &pkt->chunk_list);
>    235          pkt->size += SCTP_PAD4(ntohs(pad->chunk_hdr->length));
>    236          chunk->transport = t;
>    237
>    238          return SCTP_XMIT_OK;
>    239  }
>    240
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
David Miller June 22, 2021, 5:02 p.m. UTC | #3
From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 21 Jun 2021 21:13:59 -0400

> This is a "set but not used" warning, I can fix it after.

Please fix it now and respin, thank you.
diff mbox series

Patch

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index 45542e2bac93..2eb6d7c2c931 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -226,7 +226,8 @@  struct sctp_chunk *sctp_make_new_encap_port(
 					const struct sctp_association *asoc,
 					const struct sctp_chunk *chunk);
 struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
-				       const struct sctp_transport *transport);
+				       const struct sctp_transport *transport,
+				       __u32 probe_size);
 struct sctp_chunk *sctp_make_heartbeat_ack(const struct sctp_association *asoc,
 					   const struct sctp_chunk *chunk,
 					   const void *payload,
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index a3772f8ee7f6..f7b056f5af37 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -386,6 +386,7 @@  struct sctp_sender_hb_info {
 	union sctp_addr daddr;
 	unsigned long sent_at;
 	__u64 hb_nonce;
+	__u32 probe_size;
 };
 
 int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
@@ -657,6 +658,7 @@  struct sctp_chunk {
 		data_accepted:1,	/* At least 1 chunk accepted */
 		auth:1,			/* IN: was auth'ed | OUT: needs auth */
 		has_asconf:1,		/* IN: have seen an asconf before */
+		pmtu_probe:1,		/* Used by PLPMTUD, can be set in s HB chunk */
 		tsn_missing_report:2,	/* Data chunk missing counter. */
 		fast_retransmit:2;	/* Is this chunk fast retransmitted? */
 };
diff --git a/net/sctp/output.c b/net/sctp/output.c
index a6aa17df09ef..b78d978de0e5 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -211,6 +211,33 @@  enum sctp_xmit sctp_packet_transmit_chunk(struct sctp_packet *packet,
 	return retval;
 }
 
+/* Try to bundle a pad chunk into a packet with a heartbeat chunk for PLPMTUTD probe */
+enum sctp_xmit sctp_packet_bundle_pad(struct sctp_packet *pkt, struct sctp_chunk *chunk)
+{
+	struct sctp_transport *t = pkt->transport;
+	struct sctp_chunk *pad;
+	struct sctp_sock *sp;
+	int overhead = 0;
+
+	if (!chunk->pmtu_probe)
+		return SCTP_XMIT_OK;
+
+	sp = sctp_sk(t->asoc->base.sk);
+
+	/* calculate the Padding Data size for the pad chunk */
+	overhead += sizeof(struct sctphdr) + sizeof(struct sctp_chunkhdr);
+	overhead += sizeof(struct sctp_sender_hb_info) + sizeof(struct sctp_pad_chunk);
+	pad = sctp_make_pad(t->asoc, t->pl.probe_size - overhead);
+	if (!pad)
+		return SCTP_XMIT_DELAY;
+
+	list_add_tail(&pad->list, &pkt->chunk_list);
+	pkt->size += SCTP_PAD4(ntohs(pad->chunk_hdr->length));
+	chunk->transport = t;
+
+	return SCTP_XMIT_OK;
+}
+
 /* Try to bundle an auth chunk into the packet. */
 static enum sctp_xmit sctp_packet_bundle_auth(struct sctp_packet *pkt,
 					      struct sctp_chunk *chunk)
@@ -382,6 +409,10 @@  enum sctp_xmit sctp_packet_append_chunk(struct sctp_packet *packet,
 		goto finish;
 
 	retval = __sctp_packet_append_chunk(packet, chunk);
+	if (retval != SCTP_XMIT_OK)
+		goto finish;
+
+	retval = sctp_packet_bundle_pad(packet, chunk);
 
 finish:
 	return retval;
@@ -553,7 +584,7 @@  int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
 	sk = chunk->skb->sk;
 
 	/* check gso */
-	if (packet->size > tp->pathmtu && !packet->ipfragok) {
+	if (packet->size > tp->pathmtu && !packet->ipfragok && !chunk->pmtu_probe) {
 		if (!sk_can_gso(sk)) {
 			pr_err_once("Trying to GSO but underlying device doesn't support it.");
 			goto out;
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 5cb1aa5f067b..ff47091c385e 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -769,7 +769,11 @@  static int sctp_packet_singleton(struct sctp_transport *transport,
 
 	sctp_packet_init(&singleton, transport, sport, dport);
 	sctp_packet_config(&singleton, vtag, 0);
-	sctp_packet_append_chunk(&singleton, chunk);
+	if (sctp_packet_append_chunk(&singleton, chunk) != SCTP_XMIT_OK) {
+		list_del_init(&chunk->list);
+		sctp_chunk_free(chunk);
+		return -ENOMEM;
+	}
 	return sctp_packet_transmit(&singleton, gfp);
 }
 
@@ -929,8 +933,13 @@  static void sctp_outq_flush_ctrl(struct sctp_flush_ctx *ctx)
 			one_packet = 1;
 			fallthrough;
 
-		case SCTP_CID_SACK:
 		case SCTP_CID_HEARTBEAT:
+			if (chunk->pmtu_probe) {
+				sctp_packet_singleton(ctx->transport, chunk, ctx->gfp);
+				break;
+			}
+			fallthrough;
+		case SCTP_CID_SACK:
 		case SCTP_CID_SHUTDOWN:
 		case SCTP_CID_ECN_ECNE:
 		case SCTP_CID_ASCONF:
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index e5d470cd7c40..b0eaa93a9cc6 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1160,7 +1160,8 @@  struct sctp_chunk *sctp_make_new_encap_port(const struct sctp_association *asoc,
 
 /* Make a HEARTBEAT chunk.  */
 struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
-				       const struct sctp_transport *transport)
+				       const struct sctp_transport *transport,
+				       __u32 probe_size)
 {
 	struct sctp_sender_hb_info hbinfo;
 	struct sctp_chunk *retval;
@@ -1176,6 +1177,7 @@  struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
 	hbinfo.daddr = transport->ipaddr;
 	hbinfo.sent_at = jiffies;
 	hbinfo.hb_nonce = transport->hb_nonce;
+	hbinfo.probe_size = probe_size;
 
 	/* Cast away the 'const', as this is just telling the chunk
 	 * what transport it belongs to.
@@ -1183,6 +1185,7 @@  struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
 	retval->transport = (struct sctp_transport *) transport;
 	retval->subh.hbs_hdr = sctp_addto_chunk(retval, sizeof(hbinfo),
 						&hbinfo);
+	retval->pmtu_probe = !!probe_size;
 
 nodata:
 	return retval;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 3b99eda50618..8edb9186112a 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1004,7 +1004,7 @@  static enum sctp_disposition sctp_sf_heartbeat(
 	struct sctp_chunk *reply;
 
 	/* Send a heartbeat to our peer.  */
-	reply = sctp_make_heartbeat(asoc, transport);
+	reply = sctp_make_heartbeat(asoc, transport, 0);
 	if (!reply)
 		return SCTP_DISPOSITION_NOMEM;
 
@@ -1104,8 +1104,15 @@  enum sctp_disposition sctp_sf_send_probe(struct net *net,
 					 struct sctp_cmd_seq *commands)
 {
 	struct sctp_transport *transport = (struct sctp_transport *)arg;
+	struct sctp_chunk *reply;
+
+	if (!sctp_transport_pl_enabled(transport))
+		return SCTP_DISPOSITION_CONSUME;
 
-	/* The actual handling will be performed here in a later patch. */
+	reply = sctp_make_heartbeat(asoc, transport, transport->pl.probe_size);
+	if (!reply)
+		return SCTP_DISPOSITION_NOMEM;
+	sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
 	sctp_add_cmd_sf(commands, SCTP_CMD_PROBE_TIMER_UPDATE,
 			SCTP_TRANSPORT(transport));
 
@@ -1260,6 +1267,15 @@  enum sctp_disposition sctp_sf_backbeat_8_3(struct net *net,
 	if (hbinfo->hb_nonce != link->hb_nonce)
 		return SCTP_DISPOSITION_DISCARD;
 
+	if (hbinfo->probe_size) {
+		if (hbinfo->probe_size != link->pl.probe_size ||
+		    !sctp_transport_pl_enabled(link))
+			return SCTP_DISPOSITION_DISCARD;
+
+		/* The actual handling will be performed here in a later patch. */
+		return SCTP_DISPOSITION_CONSUME;
+	}
+
 	max_interval = link->hbinterval + link->rto;
 
 	/* Check if the timestamp looks valid.  */