[ovs-dev] [nxm 03/42] xtoxll: Add byte conversions macros for use in constant expressions.

Justin Pettit jpettit at nicira.com
Thu Oct 28 13:46:02 PDT 2010


This looks fine to me, but I wouldn't expect to find long and short byte-swapping macros in a file called "xtoxll.h", which seems to indicate it would only be concerned with long long byte-swapping.

--Justin


On Oct 28, 2010, at 10:27 AM, Ben Pfaff wrote:

> ---
> lib/xtoxll.h            |   30 +++++++++++++++++++++++++++-
> tests/automake.mk       |   10 +++++++-
> tests/library.at        |    5 ++++
> tests/test-classifier.c |   33 +++++++++++--------------------
> tests/test-xtoxll.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 103 insertions(+), 24 deletions(-)
> create mode 100644 tests/test-xtoxll.c
> 
> diff --git a/lib/xtoxll.h b/lib/xtoxll.h
> index faf6c94..a3734f1 100644
> --- a/lib/xtoxll.h
> +++ b/lib/xtoxll.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008 Nicira Networks.
> + * Copyright (c) 2008, 2010 Nicira Networks.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -18,6 +18,7 @@
> 
> #include <arpa/inet.h>
> #include <sys/types.h>
> +#include <inttypes.h>
> 
> static inline uint64_t
> htonll(uint64_t n)
> @@ -31,4 +32,31 @@ ntohll(uint64_t n)
>     return htonl(1) == 1 ? n : ((uint64_t) ntohl(n) << 32) | ntohl(n >> 32);
> }
> 
> +/* These macros may substitute for htons(), htonl(), and htonll() in contexts
> + * where function calls are not allowed, such as case labels.  They should not
> + * be used elsewhere because all of them evaluate their argument many times. */
> +#ifdef WORDS_BIGENDIAN
> +#define CONSTANT_HTONS(VALUE) ((uint16_t) (VALUE))
> +#define CONSTANT_HTONL(VALUE) ((uint32_t) (VALUE))
> +#define CONSTANT_HTONLL(VALUE) ((uint64_t) (VALUE))
> +#else
> +#define CONSTANT_HTONS(VALUE)                       \
> +        (((((uint16_t) (VALUE)) & 0xff00) >> 8) |   \
> +         ((((uint16_t) (VALUE)) & 0x00ff) << 8))
> +#define CONSTANT_HTONL(VALUE)                           \
> +        (((((uint32_t) (VALUE)) & 0x000000ff) << 24) |  \
> +         ((((uint32_t) (VALUE)) & 0x0000ff00) <<  8) |  \
> +         ((((uint32_t) (VALUE)) & 0x00ff0000) >>  8) |  \
> +         ((((uint32_t) (VALUE)) & 0xff000000) >> 24))
> +#define CONSTANT_HTONLL(VALUE)                                           \
> +        (((((uint64_t) (VALUE)) & UINT64_C(0x00000000000000ff)) << 56) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x000000000000ff00)) << 40) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x0000000000ff0000)) << 24) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x00000000ff000000)) <<  8) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x000000ff00000000)) >>  8) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x0000ff0000000000)) >> 24) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0x00ff000000000000)) >> 40) | \
> +         ((((uint64_t) (VALUE)) & UINT64_C(0xff00000000000000)) >> 56))
> +#endif
> +
> #endif /* xtoxll.h */
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 1fac457..34b0a80 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -78,7 +78,8 @@ lcov_wrappers = \
> 	tests/lcov/test-timeval \
> 	tests/lcov/test-type-props \
> 	tests/lcov/test-uuid \
> -	tests/lcov/test-vconn
> +	tests/lcov/test-vconn \
> +	tests/lcov/test-xtoxll
> 
> $(lcov_wrappers): tests/lcov-wrapper.in
> 	@test -d tests/lcov || mkdir tests/lcov
> @@ -126,7 +127,8 @@ valgrind_wrappers = \
> 	tests/valgrind/test-timeval \
> 	tests/valgrind/test-type-props \
> 	tests/valgrind/test-uuid \
> -	tests/valgrind/test-vconn
> +	tests/valgrind/test-vconn \
> +	tests/valgrind/test-xtoxll
> 
> $(valgrind_wrappers): tests/valgrind-wrapper.in
> 	@test -d tests/valgrind || mkdir tests/valgrind
> @@ -268,6 +270,10 @@ EXTRA_DIST += \
> 	tests/testpki-req.pem \
> 	tests/testpki-req2.pem
> 
> +noinst_PROGRAMS += tests/test-xtoxll
> +tests_test_xtoxll_SOURCES = tests/test-xtoxll.c
> +tests_test_xtoxll_LDADD = lib/libopenvswitch.a
> +
> # Python tests.
> EXTRA_DIST += \
> 	tests/test-daemon.py \
> diff --git a/tests/library.at b/tests/library.at
> index f9f97f1..cf7505c 100644
> --- a/tests/library.at
> +++ b/tests/library.at
> @@ -33,3 +33,8 @@ AT_CLEANUP
> AT_SETUP([test strtok_r bug fix])
> AT_CHECK([test-strtok_r], [0], [ignore])
> AT_CLEANUP
> +
> +AT_SETUP([test byte order conversion])
> +AT_KEYWORDS([xtoxll])
> +AT_CHECK([test-xtoxll], [0], [ignore])
> +AT_CLEANUP
> diff --git a/tests/test-classifier.c b/tests/test-classifier.c
> index e0a3e96..73229c2 100644
> --- a/tests/test-classifier.c
> +++ b/tests/test-classifier.c
> @@ -32,6 +32,7 @@
> #include "command-line.h"
> #include "flow.h"
> #include "packets.h"
> +#include "xtoxll.h"
> 
> #undef NDEBUG
> #include <assert.h>
> @@ -221,30 +222,20 @@ tcls_delete_matches(struct tcls *cls,
>     }
> }
> 
> -#ifdef WORDS_BIGENDIAN
> -#define T_HTONL(VALUE) ((uint32_t) (VALUE))
> -#define T_HTONS(VALUE) ((uint32_t) (VALUE))
> -#else
> -#define T_HTONL(VALUE) (((((uint32_t) (VALUE)) & 0x000000ff) << 24) | \
> -                      ((((uint32_t) (VALUE)) & 0x0000ff00) <<  8) | \
> -                      ((((uint32_t) (VALUE)) & 0x00ff0000) >>  8) | \
> -                      ((((uint32_t) (VALUE)) & 0xff000000) >> 24))
> -#define T_HTONS(VALUE) (((((uint16_t) (VALUE)) & 0xff00) >> 8) |  \
> -                      ((((uint16_t) (VALUE)) & 0x00ff) << 8))
> -#endif
> -
> -static uint32_t nw_src_values[] = { T_HTONL(0xc0a80001),
> -                                    T_HTONL(0xc0a04455) };
> -static uint32_t nw_dst_values[] = { T_HTONL(0xc0a80002),
> -                                    T_HTONL(0xc0a04455) };
> +static uint32_t nw_src_values[] = { CONSTANT_HTONL(0xc0a80001),
> +                                    CONSTANT_HTONL(0xc0a04455) };
> +static uint32_t nw_dst_values[] = { CONSTANT_HTONL(0xc0a80002),
> +                                    CONSTANT_HTONL(0xc0a04455) };
> static uint32_t tun_id_values[] = { 0, 0xffff0000 };
> -static uint16_t in_port_values[] = { T_HTONS(1), T_HTONS(OFPP_LOCAL) };
> -static uint16_t dl_vlan_values[] = { T_HTONS(101), T_HTONS(0) };
> +static uint16_t in_port_values[] = { CONSTANT_HTONS(1),
> +                                     CONSTANT_HTONS(OFPP_LOCAL) };
> +static uint16_t dl_vlan_values[] = { CONSTANT_HTONS(101), CONSTANT_HTONS(0) };
> static uint8_t dl_vlan_pcp_values[] = { 7, 0 };
> static uint16_t dl_type_values[]
> -            = { T_HTONS(ETH_TYPE_IP), T_HTONS(ETH_TYPE_ARP) };
> -static uint16_t tp_src_values[] = { T_HTONS(49362), T_HTONS(80) };
> -static uint16_t tp_dst_values[] = { T_HTONS(6667), T_HTONS(22) };
> +            = { CONSTANT_HTONS(ETH_TYPE_IP), CONSTANT_HTONS(ETH_TYPE_ARP) };
> +static uint16_t tp_src_values[] = { CONSTANT_HTONS(49362),
> +                                    CONSTANT_HTONS(80) };
> +static uint16_t tp_dst_values[] = { CONSTANT_HTONS(6667), CONSTANT_HTONS(22) };
> static uint8_t dl_src_values[][6] = { { 0x00, 0x02, 0xe3, 0x0f, 0x80, 0xa4 },
>                                       { 0x5e, 0x33, 0x7f, 0x5f, 0x1e, 0x99 } };
> static uint8_t dl_dst_values[][6] = { { 0x4a, 0x27, 0x71, 0xae, 0x64, 0xc1 },
> diff --git a/tests/test-xtoxll.c b/tests/test-xtoxll.c
> new file mode 100644
> index 0000000..b658b86
> --- /dev/null
> +++ b/tests/test-xtoxll.c
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2010 Nicira Networks.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "xtoxll.h"
> +#include <assert.h>
> +#include <inttypes.h>
> +
> +int
> +main(void)
> +{
> +    /* I picked some random numbers. */
> +    const uint16_t s = 0xc9bd;
> +    const uint32_t l = 0xffe56ae8;
> +    const uint64_t ll = UINT64_C(0xb6fe878a9117ecdb);
> +
> +    assert(htons(ntohs(s)) == s);
> +    assert(ntohs(htons(s)) == s);
> +    assert(CONSTANT_HTONS(ntohs(s)) == s);
> +    assert(ntohs(CONSTANT_HTONS(s)) == s);
> +    assert(ntohs(CONSTANT_HTONS(l)) == (uint16_t) l);
> +    assert(ntohs(CONSTANT_HTONS(ll)) == (uint16_t) ll);
> +
> +    assert(htonl(ntohl(l)) == l);
> +    assert(ntohl(htonl(l)) == l);
> +    assert(CONSTANT_HTONL(ntohl(l)) == l);
> +    assert(ntohl(CONSTANT_HTONL(l)) == l);
> +    assert(ntohl(CONSTANT_HTONL(ll)) == (uint32_t) ll);
> +
> +    assert(htonll(ntohll(ll)) == ll);
> +    assert(ntohll(htonll(ll)) == ll);
> +    assert(CONSTANT_HTONLL(ntohll(ll)) == ll);
> +    assert(ntohll(CONSTANT_HTONLL(ll)));
> +
> +    return 0;
> +}
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list