Whatever we use, it needs to be able to reference the TCP header length
field.

On Thu, Nov 27, 2014 at 1:11 PM, Maxim Uvarov <[email protected]>
wrote:

> On 11/27/2014 08:46 PM, Bala Manoharan wrote:
>
>> 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.
>>
>
> and ip.h and ipsec.h but it will require some rework of our apps. So it it
> might be better to do it after 1.0.
>
> However I think it's better to have previous Balas version with bitfields
> and add BE support for bitfield there.
>
> Maxim.
>
>
>
>
>
>> 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]
>> <mailto:[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 {
>>     __be16source;
>>     __be16dest;
>>     __be32seq;
>>     __be32ack_seq;
>>     #if defined(__LITTLE_ENDIAN_BITFIELD)
>>     __u16res1:4,
>>     doff:4,
>>     fin:1,
>>     syn:1,
>>     rst:1,
>>     psh:1,
>>     ack:1,
>>     urg:1,
>>     ece:1,
>>     cwr:1;
>>     #elif defined(__BIG_ENDIAN_BITFIELD)
>>     __u16doff: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
>>     __be16window;
>>     __sum16check;
>>     __be16urg_ptr;
>>     };
>>
>>
>>
>>     On Thu, Nov 27, 2014 at 11:32 AM, Balasubramanian Manoharan
>>     <[email protected] <mailto:[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] <mailto:[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] <mailto:[email protected]>
>>         http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>>
>>
>>
>> _______________________________________________
>> 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
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to