* [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(ð_multicast, ð_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(ð_multicast, ð_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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).