DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] examples/ptpclient: fix delay request message
@ 2021-11-17  6:18 vanshika.shukla
  2021-11-17  7:43 ` David Marchand
  2021-11-22  7:31 ` [PATCH v2] " vanshika.shukla
  0 siblings, 2 replies; 6+ messages in thread
From: vanshika.shukla @ 2021-11-17  6:18 UTC (permalink / raw)
  To: dev; +Cc: Vanshika Shukla

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.

Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>
---
 examples/ptpclient/ptpclient.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 4f32ade7fb..1eb813ab01 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -422,7 +422,7 @@ 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);
 		created_pkt->data_len = pkt_size;
 		created_pkt->pkt_len = pkt_size;
 		eth_hdr = rte_pktmbuf_mtod(created_pkt, struct rte_ether_hdr *);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] examples/ptpclient: fix delay request message
  2021-11-17  6:18 [PATCH] examples/ptpclient: fix delay request message vanshika.shukla
@ 2021-11-17  7:43 ` David Marchand
  2021-11-22  7:31 ` [PATCH v2] " vanshika.shukla
  1 sibling, 0 replies; 6+ messages in thread
From: David Marchand @ 2021-11-17  7:43 UTC (permalink / raw)
  To: Kirill Rybalchenko, Vanshika Shukla; +Cc: dev

Hello,

On Wed, Nov 17, 2021 at 7:19 AM <vanshika.shukla@nxp.com> wrote:
>
> 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.

We need a Fixes: line and it is likely a candidate for backport.

>
> Signed-off-by: Vanshika Shukla <vanshika.shukla@nxp.com>
> ---
>  examples/ptpclient/ptpclient.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
> index 4f32ade7fb..1eb813ab01 100644
> --- a/examples/ptpclient/ptpclient.c
> +++ b/examples/ptpclient/ptpclient.c
> @@ -422,7 +422,7 @@ 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);
>                 created_pkt->data_len = pkt_size;
>                 created_pkt->pkt_len = pkt_size;

This example code is in a bad shape.

Directly updating mbuf fields without caring for available data size
is a bad practice, plus there is no check on available size in
allocated buffer.
It should be like (untested):

                created_pkt = rte_pktmbuf_alloc(mbuf_pool);
                pkt_size = sizeof(struct rte_ether_hdr) +
-                       sizeof(struct ptp_message);
-               created_pkt->data_len = pkt_size;
-               created_pkt->pkt_len = pkt_size;
+                       sizeof(struct delay_req_msg);
+               if (rte_pktmbuf_append(created_pkt, pkt_size) == NULL) {
+                       rte_pktmbuf_free(created_pkt);
+                       return;
+               }
+

After this, the rest of the code uses a struct ptp_msg pointer.
It should use a pointer with the right type.

Something like:

+               struct delay_req_msg *req_msg;
...
-               ptp_msg = (struct ptp_message *)
-                       (rte_pktmbuf_mtod(created_pkt, char *) +
-                       sizeof(struct rte_ether_hdr));
+               req_msg = rte_pktmbuf_mtod_offset(created_pkt,
+                       struct delay_req_msg *, sizeof(struct rte_ether_hdr));

etc...


-- 
David Marchand


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] examples/ptpclient: fix delay request message
  2021-11-17  6:18 [PATCH] examples/ptpclient: fix delay request message vanshika.shukla
  2021-11-17  7:43 ` David Marchand
@ 2021-11-22  7:31 ` vanshika.shukla
  2021-11-23 17:39   ` Nipun Gupta
  2021-11-24 13:52   ` David Marchand
  1 sibling, 2 replies; 6+ messages in thread
From: vanshika.shukla @ 2021-11-22  7:31 UTC (permalink / raw)
  To: dev, thomas; +Cc: nipun.gupta, david.marchand, stable, Vanshika Shukla

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] examples/ptpclient: fix delay request message
  2021-11-22  7:31 ` [PATCH v2] " vanshika.shukla
@ 2021-11-23 17:39   ` Nipun Gupta
  2021-11-24 13:48     ` David Marchand
  2021-11-24 13:52   ` David Marchand
  1 sibling, 1 reply; 6+ messages in thread
From: Nipun Gupta @ 2021-11-23 17:39 UTC (permalink / raw)
  To: Vanshika Shukla, dev, thomas; +Cc: david.marchand, stable, Vanshika Shukla

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] examples/ptpclient: fix delay request message
  2021-11-23 17:39   ` Nipun Gupta
@ 2021-11-24 13:48     ` David Marchand
  0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2021-11-24 13:48 UTC (permalink / raw)
  To: Nipun Gupta; +Cc: Vanshika Shukla, dev, thomas, stable

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] examples/ptpclient: fix delay request message
  2021-11-22  7:31 ` [PATCH v2] " vanshika.shukla
  2021-11-23 17:39   ` Nipun Gupta
@ 2021-11-24 13:52   ` David Marchand
  1 sibling, 0 replies; 6+ messages in thread
From: David Marchand @ 2021-11-24 13:52 UTC (permalink / raw)
  To: Vanshika Shukla, Kirill Rybalchenko
  Cc: dev, Thomas Monjalon, Nipun Gupta, dpdk stable

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.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-24 13:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17  6:18 [PATCH] examples/ptpclient: fix delay request message vanshika.shukla
2021-11-17  7:43 ` David Marchand
2021-11-22  7:31 ` [PATCH v2] " vanshika.shukla
2021-11-23 17:39   ` Nipun Gupta
2021-11-24 13:48     ` David Marchand
2021-11-24 13:52   ` David Marchand

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git