| Message ID | 1417614603-938-1-git-send-email-bala.manoharan@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 3, 2014 at 7:50 AM, Balasubramanian Manoharan < bala.manoharan@linaro.org> wrote: > This patch adds TCP header description structure odph_tcp.h > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > helper/include/odph_tcp.h | 58 > +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > create mode 100644 helper/include/odph_tcp.h > > diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h > new file mode 100644 > index 0000000..429beed > --- /dev/null > +++ b/helper/include/odph_tcp.h > @@ -0,0 +1,58 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > + > +/** > + * @file > + * > + * ODP TCP header > + */ > + > +#ifndef ODPH_TCP_H_ > +#define ODPH_TCP_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include <odp_align.h> > +#include <odp_debug.h> > +#include <odp_byteorder.h> > + > +/** TCP header */ > +typedef struct ODP_PACKED { > + uint16be_t src_port; /**< Source port */ > + uint16be_t dst_port; /**< Destinatino port */ > + uint32be_t seq_no; /**< Sequence number */ > + uint32be_t ack_no; /**< Acknowledgment number */ > + union { > + uint32be_t flags_and_window; > + struct { > + uint32be_t rsvd1:8; > + uint32be_t flags:8; /**< TCP flags as a byte */ > + uint32be_t rsvd2:16; > + }; > + struct { > + uint32be_t hl:4; /**< Hdr len, in words */ > + uint32be_t rsvd3:6; /**< Reserved */ > + uint32be_t urg:1; /**< ACK */ > + uint32be_t ack:1; > + uint32be_t psh:1; > + uint32be_t rst:1; > + uint32be_t syn:1; > + uint32be_t fin:1; > + uint32be_t window:16; /**< Window size */ > + }; > + }; > + uint16be_t cksm; /**< Checksum */ > + uint16be_t urgptr; /**< Urgent pointer */ > +} odph_tcphdr_t; > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 3 December 2014 at 05:50, Balasubramanian Manoharan <bala.manoharan@linaro.org> wrote: > This patch adds TCP header description structure odph_tcp.h > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > --- > helper/include/odph_tcp.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > create mode 100644 helper/include/odph_tcp.h > > diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h > new file mode 100644 > index 0000000..429beed > --- /dev/null > +++ b/helper/include/odph_tcp.h > @@ -0,0 +1,58 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > + > +/** > + * @file > + * > + * ODP TCP header > + */ > + > +#ifndef ODPH_TCP_H_ > +#define ODPH_TCP_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include <odp_align.h> > +#include <odp_debug.h> > +#include <odp_byteorder.h> > + > +/** TCP header */ > +typedef struct ODP_PACKED { > + uint16be_t src_port; /**< Source port */ > + uint16be_t dst_port; /**< Destinatino port */ > + uint32be_t seq_no; /**< Sequence number */ > + uint32be_t ack_no; /**< Acknowledgment number */ > + union { > + uint32be_t flags_and_window; > + struct { > + uint32be_t rsvd1:8; > + uint32be_t flags:8; /**< TCP flags as a byte */ > + uint32be_t rsvd2:16; > + }; > + struct { > + uint32be_t hl:4; /**< Hdr len, in words */ > + uint32be_t rsvd3:6; /**< Reserved */ > + uint32be_t urg:1; /**< ACK */ > + uint32be_t ack:1; > + uint32be_t psh:1; > + uint32be_t rst:1; > + uint32be_t syn:1; > + uint32be_t fin:1; > + uint32be_t window:16; /**< Window size */ Was above tested on little endian system? It seems that above layout covers only big endian CPU. For example check 'struct tcphdr' in Linux kernel and see how it handles bit fields for different CPU endianness. And what is rational of doing such definition? Why mentioned 'struct tcphdr' from /usr/include/linux/tcp.h cannot be used instead? If it was discussed before, it is good idea to mention such rational in commit comment. Thanks, Victor > + }; > + }; > + uint16be_t cksm; /**< Checksum */ > + uint16be_t urgptr; /**< Urgent pointer */ > +} odph_tcphdr_t; > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
The problem with using the Linux file directly is that ODP is BSD and the Linux code is GPL. If we're going to 'borrow' a file FreeBSD would be a better source. Key is that for bare-metal implementations we can't assume the availability of other libraries for these sort of things. Bill On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky <victor.kamensky@linaro.org> wrote: > On 3 December 2014 at 05:50, Balasubramanian Manoharan > <bala.manoharan@linaro.org> wrote: > > This patch adds TCP header description structure odph_tcp.h > > > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > --- > > helper/include/odph_tcp.h | 58 > +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > create mode 100644 helper/include/odph_tcp.h > > > > diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h > > new file mode 100644 > > index 0000000..429beed > > --- /dev/null > > +++ b/helper/include/odph_tcp.h > > @@ -0,0 +1,58 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > + > > +/** > > + * @file > > + * > > + * ODP TCP header > > + */ > > + > > +#ifndef ODPH_TCP_H_ > > +#define ODPH_TCP_H_ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include <odp_align.h> > > +#include <odp_debug.h> > > +#include <odp_byteorder.h> > > + > > +/** TCP header */ > > +typedef struct ODP_PACKED { > > + uint16be_t src_port; /**< Source port */ > > + uint16be_t dst_port; /**< Destinatino port */ > > + uint32be_t seq_no; /**< Sequence number */ > > + uint32be_t ack_no; /**< Acknowledgment number */ > > + union { > > + uint32be_t flags_and_window; > > + struct { > > + uint32be_t rsvd1:8; > > + uint32be_t flags:8; /**< TCP flags as a byte */ > > + uint32be_t rsvd2:16; > > + }; > > + struct { > > + uint32be_t hl:4; /**< Hdr len, in words */ > > + uint32be_t rsvd3:6; /**< Reserved */ > > + uint32be_t urg:1; /**< ACK */ > > + uint32be_t ack:1; > > + uint32be_t psh:1; > > + uint32be_t rst:1; > > + uint32be_t syn:1; > > + uint32be_t fin:1; > > + uint32be_t window:16; /**< Window size */ > > Was above tested on little endian system? It seems that > above layout covers only big endian CPU. For example > check 'struct tcphdr' in Linux kernel and see how it handles > bit fields for different CPU endianness. > > And what is rational of doing such definition? > Why mentioned 'struct tcphdr' from /usr/include/linux/tcp.h > cannot be used instead? If it was discussed before, it > is good idea to mention such rational in commit comment. > > Thanks, > Victor > > > + }; > > + }; > > + uint16be_t cksm; /**< Checksum */ > > + uint16be_t urgptr; /**< Urgent pointer */ > > +} odph_tcphdr_t; > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif > > -- > > 2.0.1.472.g6f92e5f > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 3 December 2014 at 14:30, Bill Fischofer <bill.fischofer@linaro.org> wrote: > The problem with using the Linux file directly is that ODP is BSD and the > Linux code is GPL. If we're going to 'borrow' a file FreeBSD would be a > better source. Key is that for bare-metal implementations we can't assume > the availability of other libraries for these sort of things. I did not suggest to copy it from Linux. I was asking why we cannot use Linux header file directly instead. If rational to make it available for bare-metal implementations, maybe it is good idea to mention that in commit comment. Thanks, Victor > > Bill > > On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky <victor.kamensky@linaro.org> > wrote: >> >> On 3 December 2014 at 05:50, Balasubramanian Manoharan >> <bala.manoharan@linaro.org> wrote: >> > This patch adds TCP header description structure odph_tcp.h >> > >> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> > --- >> > helper/include/odph_tcp.h | 58 >> > +++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 58 insertions(+) >> > create mode 100644 helper/include/odph_tcp.h >> > >> > diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h >> > new file mode 100644 >> > index 0000000..429beed >> > --- /dev/null >> > +++ b/helper/include/odph_tcp.h >> > @@ -0,0 +1,58 @@ >> > +/* Copyright (c) 2014, Linaro Limited >> > + * All rights reserved. >> > + * >> > + * SPDX-License-Identifier: BSD-3-Clause >> > + */ >> > + >> > + >> > +/** >> > + * @file >> > + * >> > + * ODP TCP header >> > + */ >> > + >> > +#ifndef ODPH_TCP_H_ >> > +#define ODPH_TCP_H_ >> > + >> > +#ifdef __cplusplus >> > +extern "C" { >> > +#endif >> > + >> > +#include <odp_align.h> >> > +#include <odp_debug.h> >> > +#include <odp_byteorder.h> >> > + >> > +/** TCP header */ >> > +typedef struct ODP_PACKED { >> > + uint16be_t src_port; /**< Source port */ >> > + uint16be_t dst_port; /**< Destinatino port */ >> > + uint32be_t seq_no; /**< Sequence number */ >> > + uint32be_t ack_no; /**< Acknowledgment number */ >> > + union { >> > + uint32be_t flags_and_window; >> > + struct { >> > + uint32be_t rsvd1:8; >> > + uint32be_t flags:8; /**< TCP flags as a byte */ >> > + uint32be_t rsvd2:16; >> > + }; >> > + struct { >> > + uint32be_t hl:4; /**< Hdr len, in words */ >> > + uint32be_t rsvd3:6; /**< Reserved */ >> > + uint32be_t urg:1; /**< ACK */ >> > + uint32be_t ack:1; >> > + uint32be_t psh:1; >> > + uint32be_t rst:1; >> > + uint32be_t syn:1; >> > + uint32be_t fin:1; >> > + uint32be_t window:16; /**< Window size */ >> >> Was above tested on little endian system? It seems that >> above layout covers only big endian CPU. For example >> check 'struct tcphdr' in Linux kernel and see how it handles >> bit fields for different CPU endianness. >> >> And what is rational of doing such definition? >> Why mentioned 'struct tcphdr' from /usr/include/linux/tcp.h >> cannot be used instead? If it was discussed before, it >> is good idea to mention such rational in commit comment. >> >> Thanks, >> Victor >> >> > + }; >> > + }; >> > + uint16be_t cksm; /**< Checksum */ >> > + uint16be_t urgptr; /**< Urgent pointer */ >> > +} odph_tcphdr_t; >> > + >> > +#ifdef __cplusplus >> > +} >> > +#endif >> > + >> > +#endif >> > -- >> > 2.0.1.472.g6f92e5f >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 12/04/2014 02:20 AM, Victor Kamensky wrote: > On 3 December 2014 at 14:30, Bill Fischofer <bill.fischofer@linaro.org> wrote: >> The problem with using the Linux file directly is that ODP is BSD and the >> Linux code is GPL. If we're going to 'borrow' a file FreeBSD would be a >> better source. Key is that for bare-metal implementations we can't assume >> the availability of other libraries for these sort of things. > I did not suggest to copy it from Linux. I was asking why > we cannot use Linux header file directly instead. If rational > to make it available for bare-metal implementations, maybe > it is good idea to mention that in commit comment. > > Thanks, > Victor Victor, we stying with current code for protocols. I think Bala will add also ifdef for BE. Just because current code is already done and be in time. Next steps will be include linux tcp.h, ip.h, udp.h, ipsec.h from linux system includes and remove it from odp helpers. But that is additional work which can go as separate patch because it will touch all our examples, apps, tests and other functions where we used helper. Bala this version also does not include LE/BE ifdef for bitfield, but it has to if I'm right. That need to be solved before accepting this patch. Maxim. > >> Bill >> >> On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky <victor.kamensky@linaro.org> >> wrote: >>> On 3 December 2014 at 05:50, Balasubramanian Manoharan >>> <bala.manoharan@linaro.org> wrote: >>>> This patch adds TCP header description structure odph_tcp.h >>>> >>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>>> --- >>>> helper/include/odph_tcp.h | 58 >>>> +++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 58 insertions(+) >>>> create mode 100644 helper/include/odph_tcp.h >>>> >>>> diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h >>>> new file mode 100644 >>>> index 0000000..429beed >>>> --- /dev/null >>>> +++ b/helper/include/odph_tcp.h >>>> @@ -0,0 +1,58 @@ >>>> +/* Copyright (c) 2014, Linaro Limited >>>> + * All rights reserved. >>>> + * >>>> + * SPDX-License-Identifier: BSD-3-Clause >>>> + */ >>>> + >>>> + >>>> +/** >>>> + * @file >>>> + * >>>> + * ODP TCP header >>>> + */ >>>> + >>>> +#ifndef ODPH_TCP_H_ >>>> +#define ODPH_TCP_H_ >>>> + >>>> +#ifdef __cplusplus >>>> +extern "C" { >>>> +#endif >>>> + >>>> +#include <odp_align.h> >>>> +#include <odp_debug.h> >>>> +#include <odp_byteorder.h> >>>> + >>>> +/** TCP header */ >>>> +typedef struct ODP_PACKED { >>>> + uint16be_t src_port; /**< Source port */ >>>> + uint16be_t dst_port; /**< Destinatino port */ >>>> + uint32be_t seq_no; /**< Sequence number */ >>>> + uint32be_t ack_no; /**< Acknowledgment number */ >>>> + union { >>>> + uint32be_t flags_and_window; >>>> + struct { >>>> + uint32be_t rsvd1:8; >>>> + uint32be_t flags:8; /**< TCP flags as a byte */ >>>> + uint32be_t rsvd2:16; >>>> + }; >>>> + struct { >>>> + uint32be_t hl:4; /**< Hdr len, in words */ >>>> + uint32be_t rsvd3:6; /**< Reserved */ >>>> + uint32be_t urg:1; /**< ACK */ >>>> + uint32be_t ack:1; >>>> + uint32be_t psh:1; >>>> + uint32be_t rst:1; >>>> + uint32be_t syn:1; >>>> + uint32be_t fin:1; >>>> + uint32be_t window:16; /**< Window size */ >>> Was above tested on little endian system? It seems that >>> above layout covers only big endian CPU. For example >>> check 'struct tcphdr' in Linux kernel and see how it handles >>> bit fields for different CPU endianness. >>> >>> And what is rational of doing such definition? >>> Why mentioned 'struct tcphdr' from /usr/include/linux/tcp.h >>> cannot be used instead? If it was discussed before, it >>> is good idea to mention such rational in commit comment. >>> >>> Thanks, >>> Victor >>> >>>> + }; >>>> + }; >>>> + uint16be_t cksm; /**< Checksum */ >>>> + uint16be_t urgptr; /**< Urgent pointer */ >>>> +} odph_tcphdr_t; >>>> + >>>> +#ifdef __cplusplus >>>> +} >>>> +#endif >>>> + >>>> +#endif >>>> -- >>>> 2.0.1.472.g6f92e5f >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >> > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
Hi Maxim, This odph_tcp struct is used to read the values form an incoming packet as part of parsing and classification and hence the packets is always in Network Byte order. ODP application can use the inline functions odp_be_to_cpu_xx() to convert to cpu native byte order. Victor, We will add the commit message to include the rational for Bare-metal support. Regards, Bala On 4 December 2014 at 18:21, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/04/2014 02:20 AM, Victor Kamensky wrote: > >> On 3 December 2014 at 14:30, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> The problem with using the Linux file directly is that ODP is BSD and the >>> Linux code is GPL. If we're going to 'borrow' a file FreeBSD would be a >>> better source. Key is that for bare-metal implementations we can't >>> assume >>> the availability of other libraries for these sort of things. >>> >> I did not suggest to copy it from Linux. I was asking why >> we cannot use Linux header file directly instead. If rational >> to make it available for bare-metal implementations, maybe >> it is good idea to mention that in commit comment. >> >> Thanks, >> Victor >> > > Victor, we stying with current code for protocols. I think Bala will add > also ifdef for BE. Just because current code > is already done and be in time. Next steps will be include linux tcp.h, > ip.h, udp.h, ipsec.h from linux system includes and remove it > from odp helpers. But that is additional work which can go as separate > patch because it will touch all our examples, apps, tests and other > functions where we used helper. > > Bala this version also does not include LE/BE ifdef for bitfield, but it > has to if I'm right. That need to be solved before accepting this patch. > > > Maxim. > > > >> Bill >>> >>> On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky < >>> victor.kamensky@linaro.org> >>> wrote: >>> >>>> On 3 December 2014 at 05:50, Balasubramanian Manoharan >>>> <bala.manoharan@linaro.org> wrote: >>>> >>>>> This patch adds TCP header description structure odph_tcp.h >>>>> >>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>>>> --- >>>>> helper/include/odph_tcp.h | 58 >>>>> +++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 58 insertions(+) >>>>> create mode 100644 helper/include/odph_tcp.h >>>>> >>>>> diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h >>>>> new file mode 100644 >>>>> index 0000000..429beed >>>>> --- /dev/null >>>>> +++ b/helper/include/odph_tcp.h >>>>> @@ -0,0 +1,58 @@ >>>>> +/* Copyright (c) 2014, Linaro Limited >>>>> + * All rights reserved. >>>>> + * >>>>> + * SPDX-License-Identifier: BSD-3-Clause >>>>> + */ >>>>> + >>>>> + >>>>> +/** >>>>> + * @file >>>>> + * >>>>> + * ODP TCP header >>>>> + */ >>>>> + >>>>> +#ifndef ODPH_TCP_H_ >>>>> +#define ODPH_TCP_H_ >>>>> + >>>>> +#ifdef __cplusplus >>>>> +extern "C" { >>>>> +#endif >>>>> + >>>>> +#include <odp_align.h> >>>>> +#include <odp_debug.h> >>>>> +#include <odp_byteorder.h> >>>>> + >>>>> +/** TCP header */ >>>>> +typedef struct ODP_PACKED { >>>>> + uint16be_t src_port; /**< Source port */ >>>>> + uint16be_t dst_port; /**< Destinatino port */ >>>>> + uint32be_t seq_no; /**< Sequence number */ >>>>> + uint32be_t ack_no; /**< Acknowledgment number */ >>>>> + union { >>>>> + uint32be_t flags_and_window; >>>>> + struct { >>>>> + uint32be_t rsvd1:8; >>>>> + uint32be_t flags:8; /**< TCP flags as a byte */ >>>>> + uint32be_t rsvd2:16; >>>>> + }; >>>>> + struct { >>>>> + uint32be_t hl:4; /**< Hdr len, in words */ >>>>> + uint32be_t rsvd3:6; /**< Reserved */ >>>>> + uint32be_t urg:1; /**< ACK */ >>>>> + uint32be_t ack:1; >>>>> + uint32be_t psh:1; >>>>> + uint32be_t rst:1; >>>>> + uint32be_t syn:1; >>>>> + uint32be_t fin:1; >>>>> + uint32be_t window:16; /**< Window size */ >>>>> >>>> Was above tested on little endian system? It seems that >>>> above layout covers only big endian CPU. For example >>>> check 'struct tcphdr' in Linux kernel and see how it handles >>>> bit fields for different CPU endianness. >>>> >>>> And what is rational of doing such definition? >>>> Why mentioned 'struct tcphdr' from /usr/include/linux/tcp.h >>>> cannot be used instead? If it was discussed before, it >>>> is good idea to mention such rational in commit comment. >>>> >>>> Thanks, >>>> Victor >>>> >>>> + }; >>>>> + }; >>>>> + uint16be_t cksm; /**< Checksum */ >>>>> + uint16be_t urgptr; /**< Urgent pointer */ >>>>> +} odph_tcphdr_t; >>>>> + >>>>> +#ifdef __cplusplus >>>>> +} >>>>> +#endif >>>>> + >>>>> +#endif >>>>> -- >>>>> 2.0.1.472.g6f92e5f >>>>> >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 12/04/2014 03:59 PM, Bala Manoharan wrote: > Hi Maxim, > > This odph_tcp struct is used to read the values form an incoming > packet as part of parsing and classification and hence the packets is > always in Network Byte order. ODP application can use the inline > functions odp_be_to_cpu_xx() to convert to cpu native byte order. > that is right. but bits order in that bytes is different right? Maxim. > Victor, > We will add the commit message to include the rational for Bare-metal > support. > > Regards, > Bala > > On 4 December 2014 at 18:21, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 12/04/2014 02:20 AM, Victor Kamensky wrote: > > On 3 December 2014 at 14:30, Bill Fischofer > <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> > wrote: > > The problem with using the Linux file directly is that ODP > is BSD and the > Linux code is GPL. If we're going to 'borrow' a file > FreeBSD would be a > better source. Key is that for bare-metal implementations > we can't assume > the availability of other libraries for these sort of things. > > I did not suggest to copy it from Linux. I was asking why > we cannot use Linux header file directly instead. If rational > to make it available for bare-metal implementations, maybe > it is good idea to mention that in commit comment. > > Thanks, > Victor > > > Victor, we stying with current code for protocols. I think Bala > will add also ifdef for BE. Just because current code > is already done and be in time. Next steps will be include linux > tcp.h, ip.h, udp.h, ipsec.h from linux system includes and remove it > from odp helpers. But that is additional work which can go as > separate patch because it will touch all our examples, apps, tests > and other > functions where we used helper. > > Bala this version also does not include LE/BE ifdef for bitfield, > but it has to if I'm right. That need to be solved before > accepting this patch. > > > Maxim. > > > > Bill > > On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky > <victor.kamensky@linaro.org > <mailto:victor.kamensky@linaro.org>> > wrote: > > On 3 December 2014 at 05:50, Balasubramanian Manoharan > <bala.manoharan@linaro.org > <mailto:bala.manoharan@linaro.org>> wrote: > > This patch adds TCP header description structure > odph_tcp.h > > Signed-off-by: Balasubramanian Manoharan > <bala.manoharan@linaro.org > <mailto:bala.manoharan@linaro.org>> > --- > helper/include/odph_tcp.h | 58 > +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > create mode 100644 helper/include/odph_tcp.h > > diff --git a/helper/include/odph_tcp.h > b/helper/include/odph_tcp.h > new file mode 100644 > index 0000000..429beed > --- /dev/null > +++ b/helper/include/odph_tcp.h > @@ -0,0 +1,58 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > + > +/** > + * @file > + * > + * ODP TCP header > + */ > + > +#ifndef ODPH_TCP_H_ > +#define ODPH_TCP_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include <odp_align.h> > +#include <odp_debug.h> > +#include <odp_byteorder.h> > + > +/** TCP header */ > +typedef struct ODP_PACKED { > + uint16be_t src_port; /**< Source port */ > + uint16be_t dst_port; /**< Destinatino port */ > + uint32be_t seq_no; /**< Sequence number */ > + uint32be_t ack_no; /**< Acknowledgment > number */ > + union { > + uint32be_t flags_and_window; > + struct { > + uint32be_t rsvd1:8; > + uint32be_t flags:8; /**< > TCP flags as a byte */ > + uint32be_t rsvd2:16; > + }; > + struct { > + uint32be_t hl:4; /**< Hdr > len, in words */ > + uint32be_t rsvd3:6; /**< > Reserved */ > + uint32be_t urg:1; /**< ACK */ > + uint32be_t ack:1; > + uint32be_t psh:1; > + uint32be_t rst:1; > + uint32be_t syn:1; > + uint32be_t fin:1; > + uint32be_t window:16; /**< > Window size */ > > Was above tested on little endian system? It seems that > above layout covers only big endian CPU. For example > check 'struct tcphdr' in Linux kernel and see how it > handles > bit fields for different CPU endianness. > > And what is rational of doing such definition? > Why mentioned 'struct tcphdr' from > /usr/include/linux/tcp.h > cannot be used instead? If it was discussed before, it > is good idea to mention such rational in commit comment. > > Thanks, > Victor > > + }; > + }; > + uint16be_t cksm; /**< Checksum */ > + uint16be_t urgptr; /**< Urgent pointer */ > +} odph_tcphdr_t; > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 4 December 2014 at 18:46, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/04/2014 03:59 PM, Bala Manoharan wrote: > >> Hi Maxim, >> >> This odph_tcp struct is used to read the values form an incoming packet >> as part of parsing and classification and hence the packets is always in >> Network Byte order. ODP application can use the inline functions >> odp_be_to_cpu_xx() to convert to cpu native byte order. >> >> that is right. but bits order in that bytes is different right? > > Maxim. > > Yes. I need to add for the bit fields. Regards, Bala > Victor, >> We will add the commit message to include the rational for Bare-metal >> support. >> >> Regards, >> Bala >> >> On 4 December 2014 at 18:21, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> On 12/04/2014 02:20 AM, Victor Kamensky wrote: >> >> On 3 December 2014 at 14:30, Bill Fischofer >> <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> >> >> wrote: >> >> The problem with using the Linux file directly is that ODP >> is BSD and the >> Linux code is GPL. If we're going to 'borrow' a file >> FreeBSD would be a >> better source. Key is that for bare-metal implementations >> we can't assume >> the availability of other libraries for these sort of things. >> >> I did not suggest to copy it from Linux. I was asking why >> we cannot use Linux header file directly instead. If rational >> to make it available for bare-metal implementations, maybe >> it is good idea to mention that in commit comment. >> >> Thanks, >> Victor >> >> >> Victor, we stying with current code for protocols. I think Bala >> will add also ifdef for BE. Just because current code >> is already done and be in time. Next steps will be include linux >> tcp.h, ip.h, udp.h, ipsec.h from linux system includes and remove it >> from odp helpers. But that is additional work which can go as >> separate patch because it will touch all our examples, apps, tests >> and other >> functions where we used helper. >> >> Bala this version also does not include LE/BE ifdef for bitfield, >> but it has to if I'm right. That need to be solved before >> accepting this patch. >> >> >> Maxim. >> >> >> >> Bill >> >> On Wed, Dec 3, 2014 at 3:28 PM, Victor Kamensky >> <victor.kamensky@linaro.org >> <mailto:victor.kamensky@linaro.org>> >> wrote: >> >> On 3 December 2014 at 05:50, Balasubramanian Manoharan >> <bala.manoharan@linaro.org >> <mailto:bala.manoharan@linaro.org>> wrote: >> >> This patch adds TCP header description structure >> odph_tcp.h >> >> Signed-off-by: Balasubramanian Manoharan >> <bala.manoharan@linaro.org >> <mailto:bala.manoharan@linaro.org>> >> >> --- >> helper/include/odph_tcp.h | 58 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> create mode 100644 helper/include/odph_tcp.h >> >> diff --git a/helper/include/odph_tcp.h >> b/helper/include/odph_tcp.h >> new file mode 100644 >> index 0000000..429beed >> --- /dev/null >> +++ b/helper/include/odph_tcp.h >> @@ -0,0 +1,58 @@ >> +/* Copyright (c) 2014, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> + >> +/** >> + * @file >> + * >> + * ODP TCP header >> + */ >> + >> +#ifndef ODPH_TCP_H_ >> +#define ODPH_TCP_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#include <odp_align.h> >> +#include <odp_debug.h> >> +#include <odp_byteorder.h> >> + >> +/** TCP header */ >> +typedef struct ODP_PACKED { >> + uint16be_t src_port; /**< Source port */ >> + uint16be_t dst_port; /**< Destinatino port */ >> + uint32be_t seq_no; /**< Sequence number */ >> + uint32be_t ack_no; /**< Acknowledgment >> number */ >> + union { >> + uint32be_t flags_and_window; >> + struct { >> + uint32be_t rsvd1:8; >> + uint32be_t flags:8; /**< >> TCP flags as a byte */ >> + uint32be_t rsvd2:16; >> + }; >> + struct { >> + uint32be_t hl:4; /**< Hdr >> len, in words */ >> + uint32be_t rsvd3:6; /**< >> Reserved */ >> + uint32be_t urg:1; /**< ACK */ >> + uint32be_t ack:1; >> + uint32be_t psh:1; >> + uint32be_t rst:1; >> + uint32be_t syn:1; >> + uint32be_t fin:1; >> + uint32be_t window:16; /**< >> Window size */ >> >> Was above tested on little endian system? It seems that >> above layout covers only big endian CPU. For example >> check 'struct tcphdr' in Linux kernel and see how it >> handles >> bit fields for different CPU endianness. >> >> And what is rational of doing such definition? >> Why mentioned 'struct tcphdr' from >> /usr/include/linux/tcp.h >> cannot be used instead? If it was discussed before, it >> is good idea to mention such rational in commit comment. >> >> Thanks, >> Victor >> >> + }; >> + }; >> + uint16be_t cksm; /**< Checksum */ >> + uint16be_t urgptr; /**< Urgent pointer */ >> +} odph_tcphdr_t; >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif >> -- >> 2.0.1.472.g6f92e5f >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org >> > >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
diff --git a/helper/include/odph_tcp.h b/helper/include/odph_tcp.h new file mode 100644 index 0000000..429beed --- /dev/null +++ b/helper/include/odph_tcp.h @@ -0,0 +1,58 @@ +/* Copyright (c) 2014, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + + +/** + * @file + * + * ODP TCP header + */ + +#ifndef ODPH_TCP_H_ +#define ODPH_TCP_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +#include <odp_align.h> +#include <odp_debug.h> +#include <odp_byteorder.h> + +/** TCP header */ +typedef struct ODP_PACKED { + uint16be_t src_port; /**< Source port */ + uint16be_t dst_port; /**< Destinatino port */ + uint32be_t seq_no; /**< Sequence number */ + uint32be_t ack_no; /**< Acknowledgment number */ + union { + uint32be_t flags_and_window; + struct { + uint32be_t rsvd1:8; + uint32be_t flags:8; /**< TCP flags as a byte */ + uint32be_t rsvd2:16; + }; + struct { + uint32be_t hl:4; /**< Hdr len, in words */ + uint32be_t rsvd3:6; /**< Reserved */ + uint32be_t urg:1; /**< ACK */ + uint32be_t ack:1; + uint32be_t psh:1; + uint32be_t rst:1; + uint32be_t syn:1; + uint32be_t fin:1; + uint32be_t window:16; /**< Window size */ + }; + }; + uint16be_t cksm; /**< Checksum */ + uint16be_t urgptr; /**< Urgent pointer */ +} odph_tcphdr_t; + +#ifdef __cplusplus +} +#endif + +#endif
This patch adds TCP header description structure odph_tcp.h Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> --- helper/include/odph_tcp.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 helper/include/odph_tcp.h