I agree the #defines are bit verbose in this patch but I tried to create the hash defines to be compatible with the current syntax in ODP header files.
Also we had a discussion today in ODP API scrum regarding including /usr/include/linux/tcp.h header file directly in linux-generic implementations rather than using a header file in helper directory. We can also get views on that approach in this wider group. Regards, Bala On 27 November 2014 at 22:43, Bill Fischofer <[email protected]> wrote: > The TCP header is defined in RFC 793 as: > > TCP Header Format > > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Source Port | Destination Port | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Sequence Number | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Acknowledgment Number | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Data | |U|A|P|R|S|F| | > | Offset| Reserved |R|C|S|S|Y|I| Window | > | | |G|K|H|T|N|N| | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Checksum | Urgent Pointer | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Options | Padding | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | data | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > TCP Header Format > > Note that one tick mark represents one bit position. > > Figure 3. > > > If we're going to provide a mapping for this in ODP, it needs to cover the > entire header. For validation, getting the 4-bit Data Offset mapped > correctly is critical, since this is needed to ensure that the packet is > well formed. That's omitted from the proposed mapping. > > The standard way of mapping the flag byte as a byte is to use #defines: > > #define FIN 0x01 > #define SYN 0x02 > #define RST 0x04 > #define PSH 0x08 > #define ACK 0x10 > #define URG 0x20 > > And then say things like > > if (flags & SYN) ... > > or > > flags = SYN+ACK; > > The proposed routines for these bit accesses is overly verbose and > awkward. > > For the bitfields, if LInux has no problem with #ifdefs to dual map based > on endianness I don't see why we shouldn't adopt the same convention. > > The Linux tcp header is in /usr/include/linux/tcp.h: > > struct tcphdr { > __be16 source; > __be16 dest; > __be32 seq; > __be32 ack_seq; > #if defined(__LITTLE_ENDIAN_BITFIELD) > __u16 res1:4, > doff:4, > fin:1, > syn:1, > rst:1, > psh:1, > ack:1, > urg:1, > ece:1, > cwr:1; > #elif defined(__BIG_ENDIAN_BITFIELD) > __u16 doff:4, > res1:4, > cwr:1, > ece:1, > urg:1, > ack:1, > psh:1, > rst:1, > syn:1, > fin:1; > #else > #error "Adjust your <asm/byteorder.h> defines" > #endif > __be16 window; > __sum16 check; > __be16 urg_ptr; > }; > > > > On Thu, Nov 27, 2014 at 11:32 AM, Balasubramanian Manoharan < > [email protected]> wrote: > >> This patch adds TCP header description struct odph_tcphdr_t >> This structure is used for accessing TCP header information from the >> packet >> >> Signed-off-by: Balasubramanian Manoharan <[email protected]> >> --- >> helper/include/odph_tcp.h | 90 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 90 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..f625ac7 >> --- /dev/null >> +++ b/helper/include/odph_tcp.h >> @@ -0,0 +1,90 @@ >> +/* 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> >> + >> +/** @internal TCP header flag FIN offset */ >> +#define ODPH_TCPHDR_FIN_OFFSET 0 >> + >> +/** @internal TCP header flag SYN offset */ >> +#define ODPH_TCPHDR_SYN_OFFSET 1 >> + >> +/** @internal TCP header flag RST offset */ >> +#define ODPH_TCPHDR_RST_OFFSET 2 >> + >> +/** @internal TCP header flag PSH offset */ >> +#define ODPH_TCPHDR_PSH_OFFSET 3 >> + >> +/** @internal TCP header flag ACK offset */ >> +#define ODPH_TCPHDR_ACK_OFFSET 4 >> + >> +/** @internal TCP header flag URG offset */ >> +#define ODPH_TCPHDR_URG_OFFSET 5 >> + >> +/** @internal TCP header flag ECE offset */ >> +#define ODPH_TCPHDR_ECE_OFFSET 6 >> + >> +/** @internal TCP header flag CWR offset */ >> +#define ODPH_TCPHDR_CWR_OFFSET 7 >> + >> +/** @internal Returns TCP header FIN flag */ >> +#define ODPH_TCPHDR_FLAGS_FIN(flags) ((flags) & (1 << >> ODPH_TCPHDR_FIN_OFFSET)) >> + >> +/** @internal Returns TCP header SYN flag */ >> +#define ODPH_TCPHDR_FLAGS_SYN(flags) ((flags) & (1 << >> ODPH_TCPHDR_SYN_OFFSET)) >> + >> +/** @internal Returns TCP header RST flag */ >> +#define ODPH_TCPHDR_FLAGS_RST(flags) ((flags) & (1 << >> ODPH_TCPHDR_RST_OFFSET)) >> + >> +/** @internal Returns TCP header PSH flag */ >> +#define ODPH_TCPHDR_FLAGS_PSH(flags) ((flags) & (1 << >> ODPH_TCPHDR_PSH_OFFSET)) >> + >> +/** @internal Returns TCP header ACK flag */ >> +#define ODPH_TCPHDR_FLAGS_ACK(flags) ((flags) & (1 << >> ODPH_TCPHDR_ACK_OFFSET)) >> + >> +/** @internal Returns TCP header URG flag */ >> +#define ODPH_TCPHDR_FLAGS_URG(flags) ((flags) & (1 << >> ODPH_TCPHDR_URG_OFFSET)) >> + >> +/** @internal Returns TCP header ECE flag */ >> +#define ODPH_TCPHDR_FLAGS_ECE(flags) ((flags) & (1 << >> ODPH_TCPHDR_ECE_OFFSET)) >> + >> +/** @internal Returns TCP header CWR flag */ >> +#define ODPH_TCPHDR_FLAGS_CWR(flags) ((flags) & (1 << >> ODPH_TCPHDR_CWR_OFFSET)) >> + >> +/** 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 */ >> + uint8_t data_offset; /**< Data offset and reserved */ >> + uint8_t flags; /**< TCP flags as a byte */ >> + uint16be_t window; /**< 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 >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
