Hi Sean,
Few comments below.
> Add support to define ipv4 and ipv6 forwarding tables
> from reading from a config file for EM with a format
> similar to l3fwd-acl one.
>
> With the removal of the hardcoded route tables for IPv4
> and IPv6 from 'l3fwd_em', these routes have been moved
> to a separate default config file for use with EM.
>
> Related l3fwd docs have been updated to relfect these
> changes.
>
> Signed-off-by: Sean Morrissey <sean.morris...@intel.com>
> Signed-off-by: Ravi Kerur <rke...@gmail.com>
> ---
> doc/guides/sample_app_ug/l3_forward.rst | 89 +++--
> examples/l3fwd/em_default_v4.cfg | 17 +
> examples/l3fwd/em_default_v6.cfg | 17 +
> examples/l3fwd/l3fwd_em.c | 473 ++++++++++++++----------
> examples/l3fwd/l3fwd_route.h | 38 +-
> 5 files changed, 412 insertions(+), 222 deletions(-)
> create mode 100644 examples/l3fwd/em_default_v4.cfg
> create mode 100644 examples/l3fwd/em_default_v6.cfg
>
...
> diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
....
> @@ -346,6 +278,178 @@ em_get_ipv6_dst_port(void *ipv6_hdr, uint16_t portid,
> void *lookup_struct)
> #include "l3fwd_em.h"
> #endif
>
> +static int
> +em_parse_v6_net(const char *in, uint32_t *v)
> +{
> + int32_t rc;
> +
> + /* get address. */
> + rc = inet_pton(AF_INET6, in, &v);
Why '&v'?
I believe it should be:
rc = inet_pton(AF_INET6, in, v);
here.
> + if (rc != 1)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int
> +em_parse_v6_rule(char *str, struct em_rule *v)
> +{
> + int i, rc;
> + uint32_t ip[4];
> + char *s, *sp, *in[CB_FLD_MAX];
> + static const char *dlm = " \t\n";
> + int dim = CB_FLD_MAX;
> + s = str;
> +
> + for (i = 0; i != dim; i++, s = NULL) {
> + in[i] = strtok_r(s, dlm, &sp);
> + if (in[i] == NULL)
> + return -EINVAL;
> + }
> +
> + ip[0] = *(v->v6_key.ip32_dst);
> + rc = em_parse_v6_net(in[CB_FLD_DST_ADDR], ip);
Doesn't look right to me.
I think it should be just:
rc = em_parse_v6_net(in[CB_FLD_DST_ADDR], v->v6_key.ip32_dst);
and we don't need local 'ip[]' at all.
> + if (rc != 0)
> + return rc;
> + ip[0] = *(v->v6_key.ip32_src);
> + rc = em_parse_v6_net(in[CB_FLD_SRC_ADDR], ip);
Same:
rc = em_parse_v6_net(in[CB_FLD_SRC_ADDR], v->v6_key.ip32_src);
> + if (rc != 0)
> + return rc;
> +
> +
> + /* source port. */
> + GET_CB_FIELD(in[CB_FLD_SRC_PORT], v->v6_key.port_src, 0, UINT16_MAX, 0);
> + /* destination port. */
> + GET_CB_FIELD(in[CB_FLD_DST_PORT], v->v6_key.port_dst, 0, UINT16_MAX, 0);
> + /* protocol. */
> + GET_CB_FIELD(in[CB_FLD_PROTO], v->v6_key.proto, 0, UINT8_MAX, 0);
> + /* out interface. */
> + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0);
> +
> + return 0;
> +}
> +
> +static int
> +em_parse_v4_rule(char *str, struct em_rule *v)
> +{
> + int i, rc;
> + char *s, *sp, *in[CB_FLD_MAX];
> + static const char *dlm = " \t\n";
> + int dim = CB_FLD_MAX;
> + s = str;
> +
> + for (i = 0; i != dim; i++, s = NULL) {
> + in[i] = strtok_r(s, dlm, &sp);
> + if (in[i] == NULL)
> + return -EINVAL;
> + }
> +
> + rc = inet_pton(AF_INET, in[CB_FLD_DST_ADDR], &(v->v4_key.ip_dst));
> + v->v4_key.ip_dst = ntohl(v->v4_key.ip_dst);
> + if (rc != 1)
> + return rc;
> +
> + rc = inet_pton(AF_INET, in[CB_FLD_SRC_ADDR], &(v->v4_key.ip_src));
> + v->v4_key.ip_src = ntohl(v->v4_key.ip_src);
> + if (rc != 1)
> + return rc;
> +
> + /* source port. */
> + GET_CB_FIELD(in[CB_FLD_SRC_PORT], v->v4_key.port_src, 0, UINT16_MAX, 0);
> + /* destination port. */
> + GET_CB_FIELD(in[CB_FLD_DST_PORT], v->v4_key.port_dst, 0, UINT16_MAX, 0);
> + /* protocol. */
> + GET_CB_FIELD(in[CB_FLD_PROTO], v->v4_key.proto, 0, UINT8_MAX, 0);
> + /* out interface. */
> + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0);
> +
> + return 0;
> +}
> +
> +static int
> +em_add_rules(const char *rule_path,
> + struct em_rule **proute_base,
> + int (*parser)(char *, struct em_rule *))
> +{
> + struct em_rule *route_rules;
> + struct em_rule *next;
> + unsigned int route_num = 0;
> + unsigned int route_cnt = 0;
> + char buff[LINE_MAX];
> + FILE *fh;
> + unsigned int i = 0, rule_size = sizeof(*next);
> + int val;
> +
> + *proute_base = NULL;
> + fh = fopen(rule_path, "rb");
> + if (fh == NULL)
> + return -EINVAL;
> +
> + while ((fgets(buff, LINE_MAX, fh) != NULL)) {
> + if (buff[0] == ROUTE_LEAD_CHAR)
> + route_num++;
> + }
> +
> + if (route_num == 0) {
> + fclose(fh);
> + return -EINVAL;
> + }
> +
> + val = fseek(fh, 0, SEEK_SET);
> + if (val < 0) {
> + fclose(fh);
> + return -EINVAL;
> + }
> +
> + route_rules = calloc(route_num, rule_size);
> +
> + if (route_rules == NULL) {
> + fclose(fh);
> + return -EINVAL;
> + }
> +
> + i = 0;
> + while (fgets(buff, LINE_MAX, fh) != NULL) {
> + i++;
> + if (is_bypass_line(buff))
> + continue;
> +
> + char s = buff[0];
> +
> + /* Route entry */
> + if (s == ROUTE_LEAD_CHAR)
> + next = &route_rules[route_cnt];
> +
> + /* Illegal line */
> + else {
> + RTE_LOG(ERR, L3FWD,
> + "%s Line %u: should start with leading "
> + "char %c\n",
> + rule_path, i, ROUTE_LEAD_CHAR);
> + fclose(fh);
> + free(route_rules);
> + return -EINVAL;
> + }
> +
> + if (parser(buff + 1, next) != 0) {
> + RTE_LOG(ERR, L3FWD,
> + "%s Line %u: parse rules error\n",
> + rule_path, i);
> + fclose(fh);
> + free(route_rules);
> + return -EINVAL;
> + }
> +
> + route_cnt++;
> + }
> +
> + fclose(fh);
> +
> + *proute_base = route_rules;
> +
> + return route_cnt;
> +}
> +
> static void
> convert_ipv4_5tuple(struct ipv4_5tuple *key1,
> union ipv4_5tuple_host *key2)
> @@ -382,122 +486,92 @@ convert_ipv6_5tuple(struct ipv6_5tuple *key1,
> #define BIT_8_TO_15 0x0000ff00
>
> static inline void
> -populate_ipv4_few_flow_into_table(const struct rte_hash *h)
> +populate_ipv4_flow_into_table(const struct rte_hash *h)
> {
> - uint32_t i;
> + int i;
> int32_t ret;
> + struct rte_eth_dev_info dev_info;
> + char srcbuf[INET6_ADDRSTRLEN];
> + char dstbuf[INET6_ADDRSTRLEN];
>
> mask0 = (rte_xmm_t){.u32 = {BIT_8_TO_15, ALL_32_BITS,
> ALL_32_BITS, ALL_32_BITS} };
>
> - for (i = 0; i < IPV4_L3FWD_EM_NUM_ROUTES; i++) {
> - struct ipv4_l3fwd_em_route entry;
> + for (i = 0; i < route_num_v4; i++) {
> + struct em_rule *entry;
> union ipv4_5tuple_host newkey;
> + struct in_addr src;
> + struct in_addr dst;
>
> - entry = ipv4_l3fwd_em_route_array[i];
> - convert_ipv4_5tuple(&entry.key, &newkey);
> + if ((1 << em_route_base_v4[i].if_out &
> + enabled_port_mask) == 0)
> + continue;
> +
> + entry = &em_route_base_v4[i];
> + convert_ipv4_5tuple(&(entry->v4_key), &newkey);
> ret = rte_hash_add_key(h, (void *) &newkey);
> if (ret < 0) {
> rte_exit(EXIT_FAILURE, "Unable to add entry %" PRIu32
> " to the l3fwd hash.\n", i);
> }
> - ipv4_l3fwd_out_if[ret] = entry.if_out;
> + ipv4_l3fwd_out_if[ret] = entry->if_out;
> + rte_eth_dev_info_get(em_route_base_v4[i].if_out,
> + &dev_info);
> +
> + src.s_addr = htonl(em_route_base_v4[i].v4_key.ip_src);
> + dst.s_addr = htonl(em_route_base_v4[i].v4_key.ip_dst);
> + printf("EM: Adding route %s %s (%d) [%s]\n",
> + inet_ntop(AF_INET, &dst, dstbuf, sizeof(dstbuf)),
> + inet_ntop(AF_INET, &src, srcbuf, sizeof(srcbuf)),
> + em_route_base_v4[i].if_out, dev_info.device->name);
> }
> printf("Hash: Adding 0x%" PRIx64 " keys\n",
> - (uint64_t)IPV4_L3FWD_EM_NUM_ROUTES);
> + (uint64_t)route_num_v4);
> }
>
> #define BIT_16_TO_23 0x00ff0000
> static inline void
> -populate_ipv6_few_flow_into_table(const struct rte_hash *h)
> +populate_ipv6_flow_into_table(const struct rte_hash *h)
> {
> - uint32_t i;
> + int i;
> int32_t ret;
> + struct rte_eth_dev_info dev_info;
> + char srcbuf[INET6_ADDRSTRLEN];
> + char dstbuf[INET6_ADDRSTRLEN];
>
> mask1 = (rte_xmm_t){.u32 = {BIT_16_TO_23, ALL_32_BITS,
> ALL_32_BITS, ALL_32_BITS} };
>
> mask2 = (rte_xmm_t){.u32 = {ALL_32_BITS, ALL_32_BITS, 0, 0} };
>
> - for (i = 0; i < IPV6_L3FWD_EM_NUM_ROUTES; i++) {
> - struct ipv6_l3fwd_em_route entry;
> + for (i = 0; i < route_num_v6; i++) {
> + struct em_rule *entry;
> union ipv6_5tuple_host newkey;
>
> - entry = ipv6_l3fwd_em_route_array[i];
> - convert_ipv6_5tuple(&entry.key, &newkey);
> + if ((1 << em_route_base_v6[i].if_out &
> + enabled_port_mask) == 0)
> + continue;
> +
> + entry = &em_route_base_v6[i];
> + convert_ipv6_5tuple(&(entry->v6_key), &newkey);
> ret = rte_hash_add_key(h, (void *) &newkey);
> if (ret < 0) {
> rte_exit(EXIT_FAILURE, "Unable to add entry %" PRIu32
> " to the l3fwd hash.\n", i);
> }
> - ipv6_l3fwd_out_if[ret] = entry.if_out;
> + ipv6_l3fwd_out_if[ret] = entry->if_out;
> + rte_eth_dev_info_get(em_route_base_v6[i].if_out,
> + &dev_info);
> +
> + printf("EM: Adding route %s %s (%d) [%s]\n",
> + inet_ntop(AF_INET6,
> em_route_base_v6[i].v6_key.ip32_dst,
> + dstbuf, sizeof(dstbuf)),
> + inet_ntop(AF_INET6, em_route_base_v6[i].v6_key.ip32_src,
> + srcbuf, sizeof(srcbuf)),
> + em_route_base_v6[i].if_out, dev_info.device->name);
I think we need to print full key here: <dest_ip,src_ip, dst_port, src_port,
proro>.
Otherwise it is sort of misleading.
Same for ipv4.
> }
> printf("Hash: Adding 0x%" PRIx64 "keys\n",
> - (uint64_t)IPV6_L3FWD_EM_NUM_ROUTES);
> -}
> -
> -#define NUMBER_PORT_USED 16
> -static inline void
> -populate_ipv4_many_flow_into_table(const struct rte_hash *h,
> - unsigned int nr_flow)
> -{
> - unsigned i;
> -
> - mask0 = (rte_xmm_t){.u32 = {BIT_8_TO_15, ALL_32_BITS,
> - ALL_32_BITS, ALL_32_BITS} };
> -
> - for (i = 0; i < nr_flow; i++) {
> - uint8_t port = i % NUMBER_PORT_USED;
> - struct ipv4_l3fwd_em_route entry;
> - union ipv4_5tuple_host newkey;
> -
> - uint8_t a = (uint8_t)((port + 1) % BYTE_VALUE_MAX);
> -
> - /* Create the ipv4 exact match flow */
> - memset(&entry, 0, sizeof(entry));
> - entry = ipv4_l3fwd_em_route_array[port];
> - entry.key.ip_dst = RTE_IPV4(198, 18, port, a);
> - convert_ipv4_5tuple(&entry.key, &newkey);
> - int32_t ret = rte_hash_add_key(h, (void *) &newkey);
> -
> - if (ret < 0)
> - rte_exit(EXIT_FAILURE, "Unable to add entry %u\n", i);
> -
> - ipv4_l3fwd_out_if[ret] = (uint8_t) entry.if_out;
> -
> - }
> - printf("Hash: Adding 0x%x keys\n", nr_flow);
> -}
> -
> -static inline void
> -populate_ipv6_many_flow_into_table(const struct rte_hash *h,
> - unsigned int nr_flow)
> -{
> - unsigned i;
> -
> - mask1 = (rte_xmm_t){.u32 = {BIT_16_TO_23, ALL_32_BITS,
> - ALL_32_BITS, ALL_32_BITS} };
> - mask2 = (rte_xmm_t){.u32 = {ALL_32_BITS, ALL_32_BITS, 0, 0} };
> -
> - for (i = 0; i < nr_flow; i++) {
> - uint8_t port = i % NUMBER_PORT_USED;
> - struct ipv6_l3fwd_em_route entry;
> - union ipv6_5tuple_host newkey;
> -
> - /* Create the ipv6 exact match flow */
> - memset(&entry, 0, sizeof(entry));
> - entry = ipv6_l3fwd_em_route_array[port];
> - entry.key.ip_dst[15] = (port + 1) % BYTE_VALUE_MAX;
> - convert_ipv6_5tuple(&entry.key, &newkey);
> - int32_t ret = rte_hash_add_key(h, (void *) &newkey);
> -
> - if (ret < 0)
> - rte_exit(EXIT_FAILURE, "Unable to add entry %u\n", i);
> -
> - ipv6_l3fwd_out_if[ret] = (uint8_t) entry.if_out;
> -
> - }
> - printf("Hash: Adding 0x%x keys\n", nr_flow);
> + (uint64_t)route_num_v6);
> }
>
> /* Requirements:
> @@ -972,17 +1046,53 @@ em_event_main_loop_tx_q_burst_vector(__rte_unused void
> *dummy)
> return 0;
> }
>
> +static void
> +free_em_routes(void)
> +{
> + free(em_route_base_v4);
> + free(em_route_base_v6);
> + em_route_base_v4 = NULL;
> + em_route_base_v6 = NULL;
> + route_num_v4 = 0;
> + route_num_v6 = 0;
> +}
> +
> /* Load rules from the input file */
> void
> read_config_files_em(void)
> {
> - /* Empty till config file support added to EM */
> + /* ipv4 check */
> + if (parm_config.rule_ipv4_name != NULL) {
> + route_num_v4 = em_add_rules(parm_config.rule_ipv4_name,
> + &em_route_base_v4, &em_parse_v4_rule);
> + if (route_num_v4 < 0) {
> + free_em_routes();
> + rte_exit(EXIT_FAILURE, "Failed to add EM IPv4 rules\n");
> + }
> + } else {
> + RTE_LOG(ERR, L3FWD, "EM IPv4 rule file not specified\n");
> + rte_exit(EXIT_FAILURE, "Failed to get valid EM options\n");
> + }
> +
> + /* ipv6 check */
> + if (parm_config.rule_ipv6_name != NULL) {
> + route_num_v6 = em_add_rules(parm_config.rule_ipv6_name,
> + &em_route_base_v6, &em_parse_v6_rule);
> + if (route_num_v6 < 0) {
> + free_em_routes();
> + rte_exit(EXIT_FAILURE, "Failed to add EM IPv6 rules\n");
> + }
> + } else {
> + RTE_LOG(ERR, L3FWD, "EM IPv6 rule file not specified\n");
> + rte_exit(EXIT_FAILURE, "Failed to get valid EM options\n");
> + }
> }
>
> /* Initialize exact match (hash) parameters. 8< */
> void
> setup_hash(const int socketid)
> {
> + printf("IPPPROTO_UDP: %d\n", IPPROTO_UDP);
Looks unrelated, pls remove.
> struct rte_hash_parameters ipv4_l3fwd_hash_params = {
> .name = NULL,
> .entries = L3FWD_HASH_ENTRIES,
> @@ -1023,35 +1133,18 @@ setup_hash(const int socketid)
> "Unable to create the l3fwd hash on socket %d\n",
> socketid);
>
