[ovs-dev] [PATCH v2] tests: Handle endianness in netlink policy test

Mike Pattrick mkp at redhat.com
Thu Nov 25 18:49:30 UTC 2021


Hello Frode,

This patch does appear to fix the test case on big-endian systems.

On Fri, 2021-11-19 at 06:08 +0100, Frode Nordahl wrote:
> The netlink policy unit test contains test fixture data that is
> subject to endianness and currently fails on big endian systems.
> 
> Add helper that ensures correct byte order for the affected data.
> 
> Fixes: bfee9f6c0115 ("netlink: Add support for parsing link layer address.")
> Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
> ---
>  tests/test-netlink-policy.c | 60 +++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/test-netlink-policy.c b/tests/test-netlink-policy.c
> index 5f2bf7101..3050faebc 100644
> --- a/tests/test-netlink-policy.c
> +++ b/tests/test-netlink-policy.c
> @@ -32,6 +32,22 @@ _nla_len(const struct nlattr *nla) {
>      return nla->nla_len - NLA_HDRLEN;
>  }
>  
> +/* The header part of the test fixture data is subject to endianness.  This
> + * helper makes sure they are put into the buffer in the right order. */
> +static void
> +prepare_ofpbuf_fixture(struct ofpbuf *buf, uint8_t *data, size_t size) {
> +    uint16_t hword;
> +
> +    ovs_assert(size > 4);
> +
> +    ofpbuf_init(buf, size);
> +    hword = data[0] | data[1] << 8;

This is pretty minor, but what do you think about using htobe16
instead?

> +    ofpbuf_put(buf, &hword, sizeof(hword));
> +    hword = data[2] | data[3] << 8;
> +    ofpbuf_put(buf, &hword, sizeof(hword));
> +    ofpbuf_put(buf, &data[4], size - sizeof(hword) * 2);
> +}
> +
>  static void
>  test_nl_policy_parse_ll_addr(struct ovs_cmdl_context *ctx OVS_UNUSED) {
>      struct nl_policy policy[] = {
> @@ -41,7 +57,7 @@ test_nl_policy_parse_ll_addr(struct ovs_cmdl_context *ctx OVS_UNUSED) {
>      struct nlattr *attrs[ARRAY_SIZE(policy)];
>      uint8_t fixture_nl_data_policy_short[] = {
>          /* too short according to policy */
> -        0x04, 0x00, 0x2a, 0x00,
> +        0x04, 0x00, 0x2a, 0x00, 0x00,
>      };
>      uint8_t fixture_nl_data_policy_long[] = {
>          /* too long according to policy */
> @@ -62,37 +78,37 @@ test_nl_policy_parse_ll_addr(struct ovs_cmdl_context *ctx OVS_UNUSED) {
>          /* valid policy but data neither eth_addr nor ib_addr */
>          0x0b, 0x00, 0x2a, 0x00, 0x00, 0x53, 0x00, 0x00, 0x00, 0x2a, 0x00,
>      };
> -    struct ofpbuf *buf;
> +    struct ofpbuf buf;
>  
>      /* confirm policy fails with too short data */
> -    buf = ofpbuf_clone_data(&fixture_nl_data_policy_short,
> -                            sizeof(fixture_nl_data_policy_short));
> -    ovs_assert(!nl_policy_parse(buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> -    ofpbuf_delete(buf);
> +    prepare_ofpbuf_fixture(&buf, fixture_nl_data_policy_short,
> +                           sizeof(fixture_nl_data_policy_short));
> +    ovs_assert(!nl_policy_parse(&buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> +    ofpbuf_uninit(&buf);
>      memset(&attrs, 0, sizeof(*attrs));
>  
>      /* confirm policy fails with too long data */
> -    buf = ofpbuf_clone_data(&fixture_nl_data_policy_long,
> -                            sizeof(fixture_nl_data_policy_long));
> -    ovs_assert(!nl_policy_parse(buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> -    ofpbuf_delete(buf);
> +    prepare_ofpbuf_fixture(&buf, fixture_nl_data_policy_long,
> +                           sizeof(fixture_nl_data_policy_long));
> +    ovs_assert(!nl_policy_parse(&buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> +    ofpbuf_uninit(&buf);
>      memset(&attrs, 0, sizeof(*attrs));
>  
>      /* confirm policy passes and interpret valid ethernet lladdr */
> -    buf = ofpbuf_clone_data(&fixture_nl_data_eth,
> -                            sizeof(fixture_nl_data_eth));
> -    ovs_assert(nl_policy_parse(buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> +    prepare_ofpbuf_fixture(&buf, fixture_nl_data_eth,
> +                           sizeof(fixture_nl_data_eth));
> +    ovs_assert(nl_policy_parse(&buf, 0, policy, attrs, ARRAY_SIZE(policy)));
>      ovs_assert((_nla_len(attrs[42]) == sizeof(struct eth_addr)));
>      struct eth_addr eth_expect = ETH_ADDR_C(00,53,00,00,00,2a);
>      struct eth_addr eth_parsed = nl_attr_get_eth_addr(attrs[42]);
>      ovs_assert((!memcmp(&eth_expect, &eth_parsed, sizeof(struct eth_addr))));
> -    ofpbuf_delete(buf);
> +    ofpbuf_uninit(&buf);
>      memset(&attrs, 0, sizeof(*attrs));
>  
>      /* confirm policy passes and interpret valid infiniband lladdr */
> -    buf = ofpbuf_clone_data(&fixture_nl_data_ib,
> -                            sizeof(fixture_nl_data_ib));
> -    ovs_assert(nl_policy_parse(buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> +    prepare_ofpbuf_fixture(&buf, fixture_nl_data_ib,
> +                           sizeof(fixture_nl_data_ib));
> +    ovs_assert(nl_policy_parse(&buf, 0, policy, attrs, ARRAY_SIZE(policy)));
>      ovs_assert((_nla_len(attrs[42]) == sizeof(struct ib_addr)));
>      struct ib_addr ib_expect = {
>              .ia = {
> @@ -102,18 +118,18 @@ test_nl_policy_parse_ll_addr(struct ovs_cmdl_context *ctx OVS_UNUSED) {
>      };
>      struct ib_addr ib_parsed = nl_attr_get_ib_addr(attrs[42]);
>      ovs_assert((!memcmp(&ib_expect, &ib_parsed, sizeof(struct eth_addr))));
> -    ofpbuf_delete(buf);
> +    ofpbuf_uninit(&buf);
>      memset(&attrs, 0, sizeof(*attrs));
>  
>      /* confirm we're able to detect invalid data that passes policy check, this
>       * can happen because the policy defines the data to be between the
>       * currently known lladdr sizes of 6 (ETH_ALEN) and 20 (INFINIBAND_ALEN) */
> -    buf = ofpbuf_clone_data(&fixture_nl_data_invalid,
> -                            sizeof(fixture_nl_data_invalid));
> -    ovs_assert(nl_policy_parse(buf, 0, policy, attrs, ARRAY_SIZE(policy)));
> +    prepare_ofpbuf_fixture(&buf, fixture_nl_data_invalid,
> +                           sizeof(fixture_nl_data_invalid));
> +    ovs_assert(nl_policy_parse(&buf, 0, policy, attrs, ARRAY_SIZE(policy)));
>      ovs_assert(_nla_len(attrs[42]) != sizeof(struct eth_addr)
>                 && _nla_len(attrs[42]) != sizeof(struct ib_addr));
> -    ofpbuf_delete(buf);
> +    ofpbuf_uninit(&buf);
>      memset(&attrs, 0, sizeof(*attrs));
>  }
>  



More information about the dev mailing list