diff mbox series

[v2] examples/ptpclient: fix delay request message

Message ID 20211122073122.10052-1-vanshika.shukla@nxp.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v2] examples/ptpclient: fix delay request message |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

vanshika.shukla@nxp.com Nov. 22, 2021, 7:31 a.m. UTC 
  From: Vanshika Shukla <vanshika.shukla@nxp.com>

The size of delay request message sent out by the DPDK
ptpclient application was observed to have extra length
than expected. Due to this, bad messages were observed
on the master side and delay response was not received.
This patch fixes this bug.

Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
Cc: stable@dpdk.org

Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>
---
Changes in v2:
 - Added a check on available size in allocated buffer
 - Created the right type of pointer when sending DELAY_REQ packet

 examples/ptpclient/ptpclient.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

Comments

Nipun Gupta Nov. 23, 2021, 5:39 p.m. UTC | #1 
Acked-by: Nipun Gupta<nipun.gupta@nxp.com>

> -----Original Message-----
> From: vanshika.shukla@nxp.com <vanshika.shukla@nxp.com>
> Sent: 22 November 2021 13:01
> To: dev@dpdk.org; thomas@monjalon.net
> Cc: Nipun Gupta <nipun.gupta@nxp.com>; david.marchand@redhat.com;
> stable@dpdk.org; Vanshika Shukla <vanshika.shukla@nxp.com>
> Subject: [PATCH v2] examples/ptpclient: fix delay request message
> 
> From: Vanshika Shukla <vanshika.shukla@nxp.com>
> 
> The size of delay request message sent out by the DPDK
> ptpclient application was observed to have extra length
> than expected. Due to this, bad messages were observed
> on the master side and delay response was not received.
> This patch fixes this bug.
> 
> Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>
> ---
> Changes in v2:
>  - Added a check on available size in allocated buffer
>  - Created the right type of pointer when sending DELAY_REQ packet
> 
>  examples/ptpclient/ptpclient.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
> index 354c7b2c90..de799f698b 100644
> --- a/examples/ptpclient/ptpclient.c
> +++ b/examples/ptpclient/ptpclient.c
> @@ -386,6 +386,7 @@ parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
>  	struct ptp_header *ptp_hdr;
>  	struct clock_id *client_clkid;
>  	struct ptp_message *ptp_msg;
> +	struct delay_req_msg *req_msg;
>  	struct rte_mbuf *created_pkt;
>  	struct tstamp *origin_tstamp;
>  	struct rte_ether_addr eth_multicast = ether_multicast;
> @@ -423,7 +424,12 @@ parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
> 
>  		created_pkt = rte_pktmbuf_alloc(mbuf_pool);
>  		pkt_size = sizeof(struct rte_ether_hdr) +
> -			sizeof(struct ptp_message);
> +			sizeof(struct delay_req_msg);
> +
> +		if (rte_pktmbuf_append(created_pkt, pkt_size) == NULL) {
> +			rte_pktmbuf_free(created_pkt);
> +			return;
> +		}
>  		created_pkt->data_len = pkt_size;
>  		created_pkt->pkt_len = pkt_size;
>  		eth_hdr = rte_pktmbuf_mtod(created_pkt, struct rte_ether_hdr
> *);
> @@ -433,22 +439,22 @@ parse_fup(struct ptpv2_data_slave_ordinary
> *ptp_data)
>  		rte_ether_addr_copy(&eth_multicast, &eth_hdr->dst_addr);
> 
>  		eth_hdr->ether_type = htons(PTP_PROTOCOL);
> -		ptp_msg = (struct ptp_message *)
> -			(rte_pktmbuf_mtod(created_pkt, char *) +
> -			sizeof(struct rte_ether_hdr));
> -
> -		ptp_msg->delay_req.hdr.seq_id = htons(ptp_data-
> >seqID_SYNC);
> -		ptp_msg->delay_req.hdr.msg_type = DELAY_REQ;
> -		ptp_msg->delay_req.hdr.ver = 2;
> -		ptp_msg->delay_req.hdr.control = 1;
> -		ptp_msg->delay_req.hdr.log_message_interval = 127;
> -		ptp_msg->delay_req.hdr.message_length =
> +		req_msg = rte_pktmbuf_mtod_offset(created_pkt,
> +			struct delay_req_msg *, sizeof(struct
> +			rte_ether_hdr));
> +
> +		req_msg->hdr.seq_id = htons(ptp_data->seqID_SYNC);
> +		req_msg->hdr.msg_type = DELAY_REQ;
> +		req_msg->hdr.ver = 2;
> +		req_msg->hdr.control = 1;
> +		req_msg->hdr.log_message_interval = 127;
> +		req_msg->hdr.message_length =
>  			htons(sizeof(struct delay_req_msg));
> -		ptp_msg->delay_req.hdr.domain_number = ptp_hdr-
> >domain_number;
> +		req_msg->hdr.domain_number = ptp_hdr->domain_number;
> 
>  		/* Set up clock id. */
>  		client_clkid =
> -			&ptp_msg->delay_req.hdr.source_port_id.clock_id;
> +			&req_msg->hdr.source_port_id.clock_id;
> 
>  		client_clkid->id[0] = eth_hdr->src_addr.addr_bytes[0];
>  		client_clkid->id[1] = eth_hdr->src_addr.addr_bytes[1];
> --
> 2.17.1
David Marchand Nov. 24, 2021, 1:48 p.m. UTC | #2 
On Tue, Nov 23, 2021 at 6:40 PM Nipun Gupta <nipun.gupta@nxp.com> wrote:
>
> Acked-by: Nipun Gupta<nipun.gupta@nxp.com>

Please don't top post.
Can you also be careful with your ack tag? It is missing a space
between name and mail address.

>
> > -----Original Message-----
> > From: vanshika.shukla@nxp.com <vanshika.shukla@nxp.com>
> > Sent: 22 November 2021 13:01
> > To: dev@dpdk.org; thomas@monjalon.net
> > Cc: Nipun Gupta <nipun.gupta@nxp.com>; david.marchand@redhat.com;
> > stable@dpdk.org; Vanshika Shukla <vanshika.shukla@nxp.com>
> > Subject: [PATCH v2] examples/ptpclient: fix delay request message
> >
> > From: Vanshika Shukla <vanshika.shukla@nxp.com>
> >
> > The size of delay request message sent out by the DPDK
> > ptpclient application was observed to have extra length
> > than expected. Due to this, bad messages were observed
> > on the master side and delay response was not received.
> > This patch fixes this bug.
> >
> > Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>
David Marchand Nov. 24, 2021, 1:52 p.m. UTC | #3 
On Mon, Nov 22, 2021 at 8:31 AM <vanshika.shukla@nxp.com> wrote:
> The size of delay request message sent out by the DPDK
> ptpclient application was observed to have extra length
> than expected. Due to this, bad messages were observed
> on the master side and delay response was not received.
> This patch fixes this bug.
>
> Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
> Cc: stable@dpdk.org
>
> Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>

Please copy relevant maintainers in addition to your colleagues.
You can use get-maintainer.sh script for this.


This fix lgtm.

Acked-by: Nipun Gupta <nipun.gupta@nxp.com>
Applied, thanks.
diff mbox series

Patch

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 354c7b2c90..de799f698b 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -386,6 +386,7 @@  parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
 	struct ptp_header *ptp_hdr;
 	struct clock_id *client_clkid;
 	struct ptp_message *ptp_msg;
+	struct delay_req_msg *req_msg;
 	struct rte_mbuf *created_pkt;
 	struct tstamp *origin_tstamp;
 	struct rte_ether_addr eth_multicast = ether_multicast;
@@ -423,7 +424,12 @@  parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
 
 		created_pkt = rte_pktmbuf_alloc(mbuf_pool);
 		pkt_size = sizeof(struct rte_ether_hdr) +
-			sizeof(struct ptp_message);
+			sizeof(struct delay_req_msg);
+
+		if (rte_pktmbuf_append(created_pkt, pkt_size) == NULL) {
+			rte_pktmbuf_free(created_pkt);
+			return;
+		}
 		created_pkt->data_len = pkt_size;
 		created_pkt->pkt_len = pkt_size;
 		eth_hdr = rte_pktmbuf_mtod(created_pkt, struct rte_ether_hdr *);
@@ -433,22 +439,22 @@  parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
 		rte_ether_addr_copy(&eth_multicast, &eth_hdr->dst_addr);
 
 		eth_hdr->ether_type = htons(PTP_PROTOCOL);
-		ptp_msg = (struct ptp_message *)
-			(rte_pktmbuf_mtod(created_pkt, char *) +
-			sizeof(struct rte_ether_hdr));
-
-		ptp_msg->delay_req.hdr.seq_id = htons(ptp_data->seqID_SYNC);
-		ptp_msg->delay_req.hdr.msg_type = DELAY_REQ;
-		ptp_msg->delay_req.hdr.ver = 2;
-		ptp_msg->delay_req.hdr.control = 1;
-		ptp_msg->delay_req.hdr.log_message_interval = 127;
-		ptp_msg->delay_req.hdr.message_length =
+		req_msg = rte_pktmbuf_mtod_offset(created_pkt,
+			struct delay_req_msg *, sizeof(struct
+			rte_ether_hdr));
+
+		req_msg->hdr.seq_id = htons(ptp_data->seqID_SYNC);
+		req_msg->hdr.msg_type = DELAY_REQ;
+		req_msg->hdr.ver = 2;
+		req_msg->hdr.control = 1;
+		req_msg->hdr.log_message_interval = 127;
+		req_msg->hdr.message_length =
 			htons(sizeof(struct delay_req_msg));
-		ptp_msg->delay_req.hdr.domain_number = ptp_hdr->domain_number;
+		req_msg->hdr.domain_number = ptp_hdr->domain_number;
 
 		/* Set up clock id. */
 		client_clkid =
-			&ptp_msg->delay_req.hdr.source_port_id.clock_id;
+			&req_msg->hdr.source_port_id.clock_id;
 
 		client_clkid->id[0] = eth_hdr->src_addr.addr_bytes[0];
 		client_clkid->id[1] = eth_hdr->src_addr.addr_bytes[1];