diff mbox series

[API-NEXT,v2,16/20] linux-generic: packet: add functions to optimize memset and memcmp paths

Message ID 1495490409-30066-17-git-send-email-odpbot@yandex.ru
State New
Headers show
Series [API-NEXT,v2,1/20] test: crypto: explicitly pass auth_digest_len to crypto subsystem | expand

Commit Message

Github ODP bot May 22, 2017, 10 p.m. UTC 
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>


Add function implementing memset and memcmp on packet object.

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

---
/** Email created from pull request 34 (lumag:crypto-update-main-new)
 ** https://github.com/Linaro/odp/pull/34
 ** Patch: https://github.com/Linaro/odp/pull/34.patch
 ** Base sha: 826ee894aa0ebd09d42a17e1de077c46bc5b366a
 ** Merge commit sha: 7c49c61063e2d57f049a5436cf12a3c36710bb34
 **/
 .../linux-generic/include/odp_packet_internal.h    |  6 +++
 platform/linux-generic/odp_packet.c                | 48 ++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Comments

Savolainen, Petri (Nokia - FI/Espoo) May 31, 2017, 12:08 p.m. UTC | #1 
> diff --git a/platform/linux-generic/include/odp_packet_internal.h

> b/platform/linux-generic/include/odp_packet_internal.h

> index d0db7008..a480a748 100644

> --- a/platform/linux-generic/include/odp_packet_internal.h

> +++ b/platform/linux-generic/include/odp_packet_internal.h

> @@ -237,6 +237,12 @@ int packet_parse_common(packet_parser_t *pkt_hdr,

> const uint8_t *ptr,

> 

>  int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const uint8_t *parseptr);

> 

> +int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,

> +			 uint8_t c, uint32_t len);

> +

> +int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,

> +			 const void *s, uint32_t len);

> +


Since packet_internal.h is internal header and should not be visible to application, _odp prefix is not needed. Most (all recent) functions in the file do not have prefix. So, change to packet_set_data() and packet_cmp_data()

-Petri
Dmitry Eremin-Solenikov May 31, 2017, 1:18 p.m. UTC | #2 
On 31.05.2017 15:08, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 

>> diff --git a/platform/linux-generic/include/odp_packet_internal.h

>> b/platform/linux-generic/include/odp_packet_internal.h

>> index d0db7008..a480a748 100644

>> --- a/platform/linux-generic/include/odp_packet_internal.h

>> +++ b/platform/linux-generic/include/odp_packet_internal.h

>> @@ -237,6 +237,12 @@ int packet_parse_common(packet_parser_t *pkt_hdr,

>> const uint8_t *ptr,

>>

>>  int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const uint8_t *parseptr);

>>

>> +int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,

>> +			 uint8_t c, uint32_t len);

>> +

>> +int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,

>> +			 const void *s, uint32_t len);

>> +

> 

> Since packet_internal.h is internal header and should not be visible to application, _odp prefix is not needed. Most (all recent) functions in the file do not have prefix. So, change to packet_set_data() and packet_cmp_data()



These symbols still leak into static library poisoning global name. Thus
in my opinion we should limit non-static symbols to odp_ and _odp_
namespaces.


-- 
With best wishes
Dmitry
Savolainen, Petri (Nokia - FI/Espoo) May 31, 2017, 2:14 p.m. UTC | #3 
> -----Original Message-----

> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org]

> Sent: Wednesday, May 31, 2017 4:18 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>;

> Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH API-NEXT v2 16/20] linux-generic: packet:

> add functions to optimize memset and memcmp paths

> 

> On 31.05.2017 15:08, Savolainen, Petri (Nokia - FI/Espoo) wrote:

> >

> >> diff --git a/platform/linux-generic/include/odp_packet_internal.h

> >> b/platform/linux-generic/include/odp_packet_internal.h

> >> index d0db7008..a480a748 100644

> >> --- a/platform/linux-generic/include/odp_packet_internal.h

> >> +++ b/platform/linux-generic/include/odp_packet_internal.h

> >> @@ -237,6 +237,12 @@ int packet_parse_common(packet_parser_t *pkt_hdr,

> >> const uint8_t *ptr,

> >>

> >>  int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const uint8_t

> *parseptr);

> >>

> >> +int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,

> >> +			 uint8_t c, uint32_t len);

> >> +

> >> +int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,

> >> +			 const void *s, uint32_t len);

> >> +

> >

> > Since packet_internal.h is internal header and should not be visible to

> application, _odp prefix is not needed. Most (all recent) functions in the

> file do not have prefix. So, change to packet_set_data() and

> packet_cmp_data()

> 

> 

> These symbols still leak into static library poisoning global name. Thus

> in my opinion we should limit non-static symbols to odp_ and _odp_

> namespaces.

> 


OK. Will you correct all other comments, including checkpatch warnings. It would be good to get these into the next release (wrapping up this week).

-Petri
Dmitry Eremin-Solenikov May 31, 2017, 8:31 p.m. UTC | #4 
On 31.05.2017 17:14, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 

> 

>> -----Original Message-----

>> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org]

>> Sent: Wednesday, May 31, 2017 4:18 PM

>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>;

>> Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org

>> Subject: Re: [lng-odp] [PATCH API-NEXT v2 16/20] linux-generic: packet:

>> add functions to optimize memset and memcmp paths

>>

>> On 31.05.2017 15:08, Savolainen, Petri (Nokia - FI/Espoo) wrote:

>>>

>>>> diff --git a/platform/linux-generic/include/odp_packet_internal.h

>>>> b/platform/linux-generic/include/odp_packet_internal.h

>>>> index d0db7008..a480a748 100644

>>>> --- a/platform/linux-generic/include/odp_packet_internal.h

>>>> +++ b/platform/linux-generic/include/odp_packet_internal.h

>>>> @@ -237,6 +237,12 @@ int packet_parse_common(packet_parser_t *pkt_hdr,

>>>> const uint8_t *ptr,

>>>>

>>>>  int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const uint8_t

>> *parseptr);

>>>>

>>>> +int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,

>>>> +			 uint8_t c, uint32_t len);

>>>> +

>>>> +int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,

>>>> +			 const void *s, uint32_t len);

>>>> +

>>>

>>> Since packet_internal.h is internal header and should not be visible to

>> application, _odp prefix is not needed. Most (all recent) functions in the

>> file do not have prefix. So, change to packet_set_data() and

>> packet_cmp_data()

>>

>>

>> These symbols still leak into static library poisoning global name. Thus

>> in my opinion we should limit non-static symbols to odp_ and _odp_

>> namespaces.

>>

> 

> OK. Will you correct all other comments, including checkpatch warnings. It would be good to get these into the next release (wrapping up this week).



Sure!


-- 
With best wishes
Dmitry
diff mbox series

Patch

diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h
index d0db7008..a480a748 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -237,6 +237,12 @@  int packet_parse_common(packet_parser_t *pkt_hdr, const uint8_t *ptr,
 
 int _odp_cls_parse(odp_packet_hdr_t *pkt_hdr, const uint8_t *parseptr);
 
+int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,
+			 uint8_t c, uint32_t len);
+
+int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,
+			 const void *s, uint32_t len);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 9ff642be..6799c6ef 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -1634,6 +1634,54 @@  int odp_packet_move_data(odp_packet_t pkt, uint32_t dst_offset,
 					pkt, src_offset, len);
 }
 
+int _odp_packet_set_data(odp_packet_t pkt, uint32_t offset,
+			 uint8_t c, uint32_t len)
+{
+	void *mapaddr;
+	uint32_t seglen = 0; /* GCC */
+	uint32_t setlen;
+	odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
+
+	if (offset + len > pkt_hdr->frame_len)
+		return -1;
+
+	while (len > 0) {
+		mapaddr = packet_map(pkt_hdr, offset, &seglen, NULL);
+		setlen = len > seglen ? seglen : len;
+		memset(mapaddr, c, setlen);
+		offset  += setlen;
+		len     -= setlen;
+	}
+
+	return 0;
+}
+
+int _odp_packet_cmp_data(odp_packet_t pkt, uint32_t offset,
+			 const void *s, uint32_t len)
+{
+	const uint8_t *ptr = s;
+	void *mapaddr;
+	uint32_t seglen = 0; /* GCC */
+	uint32_t cmplen;
+	int ret;
+	odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
+
+	ODP_ASSERT(offset + len <= pkt_hdr->frame_len);
+
+	while (len > 0) {
+		mapaddr = packet_map(pkt_hdr, offset, &seglen, NULL);
+		cmplen = len > seglen ? seglen : len;
+		ret = memcmp(mapaddr, ptr, cmplen);
+		if (ret != 0)
+			return ret;
+		offset  += cmplen;
+		len     -= cmplen;
+		ptr     += cmplen;
+	}
+
+	return 0;
+}
+
 /*
  *
  * Debugging