Let's get some final changes, so that we can hopefully merge this patch
On 22.02.2017 15:27, Nikhil Agarwal wrote:
> Signed-off-by: Nikhil Agarwal <nikhil.agar...@linaro.org>
> ---
[skipped]
> + /* Read the source MAC address for this interface */
> + ret = odp_pktio_mac_addr(pktio, src_mac, sizeof(src_mac));
> + if (ret < 0) {
> + EXAMPLE_ABORT("Error: failed during MAC address get for %s\n",
> + intf);
> + }
> +
> + printf("Created pktio:%02" PRIu64 "\n", odp_pktio_to_u64(pktio));
This needs #include <inttypes.h>
> +
> + /* Resolve any routes using this interface for output */
> + resolve_fwd_db(intf, pktout, src_mac);
> +}
> +
[skipped]
> +
> + } else if (ODP_EVENT_IPSEC_RESULT == odp_event_type(ev)) {
> + odp_ipsec_op_result_t result;
> + odp_ipsec_packet_result_t res;
> + odph_ethhdr_t *eth;
> + odp_packet_t out_pkt;
> +
> + result.pkt = &out_pkt;
> + result.res = &res;
> +
> + if (odp_unlikely(odp_ipsec_result(&result, ev) < 0)) {
> + EXAMPLE_DBG("Error Event\n");
> + odp_packet_free((odp_packet_t)ev);
This should be odp_event_free()
> + continue;
> + }
> +
> + if (odp_unlikely(res.status.all)) {
> + odp_packet_free((odp_packet_t)ev);
I'm unsure here. event should be already freed by odp_isec_result(). So
you should only free packets from result, not the event.
> + continue;
> + }
> +
> + odph_ipv4hdr_t *ip = (odph_ipv4hdr_t
> *)odp_packet_l3_ptr(out_pkt, NULL);
> +
> + if (ip->proto != IPPROTO_ESP) {
> + rc = do_route_fwd_db(out_pkt);
> + if (odp_unlikely(rc))
> + continue;
> + }
> +
> + out_port = (odp_out_entry_t
> *)odp_packet_user_ptr(out_pkt);
> + out_queue = (odp_pktout_queue_t)out_port->pktout;
> +
> + eth = (odph_ethhdr_t *)((void *)ip -
> sizeof(odph_ethhdr_t));
Void pointer arithmetics is an extension. Could you please change that
to uint8_t *.
> + eth->dst = out_port->next_hop_addr;
> + eth->src = out_port->addr;
> + eth->type = odp_cpu_to_be_16(0x800);
> +
> + if (odp_unlikely(odp_pktout_send(out_queue, &out_pkt,
> 1) < 0))
> + odp_packet_free(out_pkt);
And here also just free packet, not an event.
> + } else {
> + EXAMPLE_DBG("Invalid Event\n");
> + odp_packet_free((odp_packet_t)ev);
And here of course just call odp_event_free(), not packet_free!
> + continue;
> + }
> + }
> +
[skipped]
> +/**
> + * Hash calculation utility
> + */
> +#define JHASH_GOLDEN_RATIO 0x9e3779b9
> +#define rot(x, k) (((x) << (k)) | ((x) >> (32 - (k))))
> +#define ODP_BJ3_MIX(a, b, c) \
This is what Maxim wrote about. Could you please rename these two
defines so that they don't pollute ODP_ namespace.
> +{ \
> + a -= c; a ^= rot(c, 4); c += b; \
> + b -= a; b ^= rot(a, 6); a += c; \
> + c -= b; c ^= rot(b, 8); b += a; \
> + a -= c; a ^= rot(c, 16); c += b; \
> + b -= a; b ^= rot(a, 19); a += c; \
> + c -= b; c ^= rot(b, 4); b += a; \
> +}
> +
> +/**
> + * Default Hash bucket number
> + */
> +#define ODP_DEFAULT_BUCKET_COUNT 1024
[skipped]
> +static inline
> +int parse_key_string(char *keystring,
> + ipsec_key_t *key,
> + ipsec_alg_t *alg)
> +{
> + int idx;
> + int key_bits_in = KEY_STR_BITS(keystring);
> + char temp[3];
> +
> + key->length = 0;
> +
> + /* Algorithm is either cipher or authentication */
> + if (alg->cipher) {
> + if ((alg->u.cipher == ODP_CIPHER_ALG_3DES_CBC) &&
> + (KEY_BITS_3DES == key_bits_in))
> + key->length = key_bits_in / 8;
> + if ((alg->u.cipher == ODP_CIPHER_ALG_AES128_CBC) &&
ODP_CIPHER_ALG_AES_CBC?
> + (KEY_BITS_AES == key_bits_in))
> + key->length = key_bits_in / 8;
> + } else {
> + if ((alg->u.auth == ODP_AUTH_ALG_MD5_96) &&
ODP_AUTH_ALG_MD5_HMAC
> + (KEY_BITS_MD5_96 == key_bits_in))
> + key->length = key_bits_in / 8;
> + if ((alg->u.auth == ODP_AUTH_ALG_SHA1_96) &&
ODP_AUTH_ALG_SHA1_HMAC
> + (KEY_BITS_SHA1_96 == key_bits_in))
> + key->length = key_bits_in / 8;
> + if ((alg->u.auth == ODP_AUTH_ALG_SHA256_128) &&
OD_AUTH_ALG_SHA256_HMAC
> + (KEY_BITS_SHA2_256 == key_bits_in))
> + key->length = key_bits_in / 8;
> + }
> +
[skipped]
> + case 2:
> + if (cipher) {
> + if (0 == strcmp(token, "3des")) {
> + entry->alg.u.cipher =
> + ODP_CIPHER_ALG_3DES_CBC;
> + } else if (0 == strcmp(token, "aes")) {
> + entry->alg.u.cipher =
> + ODP_CIPHER_ALG_AES128_CBC;
ODP_CIPHER_ALG_AES_CBC
> + } else {
> + entry->alg.u.cipher =
> + ODP_CIPHER_ALG_NULL;
> + }
> + } else {
> + if (0 == strcmp(token, "md5")) {
> + entry->alg.u.auth =
> + ODP_AUTH_ALG_MD5_96;
ODP_AUTH_ALG_MD5_HMAC
> + } else if (0 == strcmp(token, "sha1")) {
> + entry->alg.u.auth =
> + ODP_AUTH_ALG_SHA1_96;
ODP_AUTH_ALG_SHA1_HMAC
> + } else if (0 == strcmp(token, "sha256")) {
> + entry->alg.u.auth =
> + ODP_AUTH_ALG_SHA256_128;
ODP_AUTH_ALG_SHA256_HMAC
> + } else {
> + entry->alg.u.auth = ODP_AUTH_ALG_NULL;
> + }
> + }
> + break;
--
With best wishes
Dmitry
