Hi Konstantin,
Please see inline.
Thanks,
Lukasz
On 24.12.2019 14:13, Ananyev, Konstantin wrote:
> External Email
>
> ----------------------------------------------------------------------
>
>> --- a/examples/ipsec-secgw/ipsec_worker.c
>> +++ b/examples/ipsec-secgw/ipsec_worker.c
>> @@ -15,6 +15,7 @@
>> #include <ctype.h>
>> #include <stdbool.h>
>>
>> +#include <rte_acl.h>
>> #include <rte_common.h>
>> #include <rte_log.h>
>> #include <rte_memcpy.h>
>> @@ -29,12 +30,51 @@
>> #include <rte_eventdev.h>
>> #include <rte_malloc.h>
>> #include <rte_mbuf.h>
>> +#include <rte_lpm.h>
>> +#include <rte_lpm6.h>
>>
>> #include "ipsec.h"
>> +#include "ipsec_worker.h"
>> #include "event_helper.h"
>>
>> extern volatile bool force_quit;
>>
>> +static inline enum pkt_type
>> +process_ipsec_get_pkt_type(struct rte_mbuf *pkt, uint8_t **nlp)
>> +{
>> + struct rte_ether_hdr *eth;
>> +
>> + eth = rte_pktmbuf_mtod(pkt, struct rte_ether_hdr *);
>> + if (eth->ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4)) {
>> + *nlp = RTE_PTR_ADD(eth, RTE_ETHER_HDR_LEN +
>> + offsetof(struct ip, ip_p));
>> + if (**nlp == IPPROTO_ESP)
>> + return PKT_TYPE_IPSEC_IPV4;
>> + else
>> + return PKT_TYPE_PLAIN_IPV4;
>> + } else if (eth->ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV6)) {
>> + *nlp = RTE_PTR_ADD(eth, RTE_ETHER_HDR_LEN +
>> + offsetof(struct ip6_hdr, ip6_nxt));
>> + if (**nlp == IPPROTO_ESP)
>> + return PKT_TYPE_IPSEC_IPV6;
>> + else
>> + return PKT_TYPE_PLAIN_IPV6;
>> + }
>> +
>> + /* Unknown/Unsupported type */
>> + return PKT_TYPE_INVALID;
>> +}
>
> Looking though that file, it seems like you choose to create your own set of
> helper functions, instead of trying to reuse existing ones:
>
> process_ipsec_get_pkt_type() VS prepare_one_packet()
> update_mac_addrs() VS prepare_tx_pkt()
> check_sp() VS inbound_sp_sa()
>
> Obviously there is nothing good in code (and possible bugs) duplication.
> Any reason why you can't reuse existing functions and need to reinvent your
> own?
[Lukasz] The prepare_one_packet() and prepare_tx_pkt() do much more than we
need and for performance reasons
we crafted new functions. For example process_ipsec_get_pkt_type function
returns nlp and whether
packet type is plain or IPsec. That's all. Prepare_one_packet() process packets
in chunks and does much more -
it adjusts mbuf and ipv4 lengths then it demultiplex packet into plan and IPsec
flows and finally does
inline checks. This is similar for update_mac_addrs() vs prepare_tx_pkt() and
check_sp() vs inbound_sp_sa()
that prepare_tx_pkt() and inbound_sp_sa() do more that we need in event mode.
>
>> +
>> +static inline void
>> +update_mac_addrs(struct rte_mbuf *pkt, uint16_t portid)
>> +{
>> + struct rte_ether_hdr *ethhdr;
>> +
>> + ethhdr = rte_pktmbuf_mtod(pkt, struct rte_ether_hdr *);
>> + memcpy(ðhdr->s_addr, ðaddr_tbl[portid].src, RTE_ETHER_ADDR_LEN);
>> + memcpy(ðhdr->d_addr, ðaddr_tbl[portid].dst, RTE_ETHER_ADDR_LEN);
>> +}
>> +
>> static inline void
>> ipsec_event_pre_forward(struct rte_mbuf *m, unsigned int port_id)
>> {
>> @@ -45,6 +85,177 @@ ipsec_event_pre_forward(struct rte_mbuf *m, unsigned int
>> port_id)
>> rte_event_eth_tx_adapter_txq_set(m, 0);
>> }
>>
>> +static inline int
>> +check_sp(struct sp_ctx *sp, const uint8_t *nlp, uint32_t *sa_idx)
>> +{
>> + uint32_t res;
>> +
>> + if (unlikely(sp == NULL))
>> + return 0;
>> +
>> + rte_acl_classify((struct rte_acl_ctx *)sp, &nlp, &res, 1,
>> + DEFAULT_MAX_CATEGORIES);
>> +
>> + if (unlikely(res == 0)) {
>> + /* No match */
>> + return 0;
>> + }
>> +
>> + if (res == DISCARD)
>> + return 0;
>> + else if (res == BYPASS) {
>> + *sa_idx = 0;
>> + return 1;
>> + }
>> +
>> + *sa_idx = SPI2IDX(res);
>> + if (*sa_idx < IPSEC_SA_MAX_ENTRIES)
>> + return 1;
>> +
>> + /* Invalid SA IDX */
>> + return 0;
>> +}
>> +
>> +static inline uint16_t
>> +route4_pkt(struct rte_mbuf *pkt, struct rt_ctx *rt_ctx)
>> +{
>> + uint32_t dst_ip;
>> + uint16_t offset;
>> + uint32_t hop;
>> + int ret;
>> +
>> + offset = RTE_ETHER_HDR_LEN + offsetof(struct ip, ip_dst);
>> + dst_ip = *rte_pktmbuf_mtod_offset(pkt, uint32_t *, offset);
>> + dst_ip = rte_be_to_cpu_32(dst_ip);
>> +
>> + ret = rte_lpm_lookup((struct rte_lpm *)rt_ctx, dst_ip, &hop);
>> +
>> + if (ret == 0) {
>> + /* We have a hit */
>> + return hop;
>> + }
>> +
>> + /* else */
>> + return RTE_MAX_ETHPORTS;
>> +}
>> +
>> +/* TODO: To be tested */
>> +static inline uint16_t
>> +route6_pkt(struct rte_mbuf *pkt, struct rt_ctx *rt_ctx)
>> +{
>> + uint8_t dst_ip[16];
>> + uint8_t *ip6_dst;
>> + uint16_t offset;
>> + uint32_t hop;
>> + int ret;
>> +
>> + offset = RTE_ETHER_HDR_LEN + offsetof(struct ip6_hdr, ip6_dst);
>> + ip6_dst = rte_pktmbuf_mtod_offset(pkt, uint8_t *, offset);
>> + memcpy(&dst_ip[0], ip6_dst, 16);
>> +
>> + ret = rte_lpm6_lookup((struct rte_lpm6 *)rt_ctx, dst_ip, &hop);
>> +
>> + if (ret == 0) {
>> + /* We have a hit */
>> + return hop;
>> + }
>> +
>> + /* else */
>> + return RTE_MAX_ETHPORTS;
>> +}
>> +
>> +static inline uint16_t
>> +get_route(struct rte_mbuf *pkt, struct route_table *rt, enum pkt_type type)
>> +{
>> + if (type == PKT_TYPE_PLAIN_IPV4 || type == PKT_TYPE_IPSEC_IPV4)
>> + return route4_pkt(pkt, rt->rt4_ctx);
>> + else if (type == PKT_TYPE_PLAIN_IPV6 || type == PKT_TYPE_IPSEC_IPV6)
>> + return route6_pkt(pkt, rt->rt6_ctx);
>> +
>> + return RTE_MAX_ETHPORTS;
>> +}
>> +
>> +static inline int
>> +process_ipsec_ev_inbound(struct ipsec_ctx *ctx, struct route_table *rt,
>> + struct rte_event *ev)
>> +{
>> + struct ipsec_sa *sa = NULL;
>> + struct rte_mbuf *pkt;
>> + uint16_t port_id = 0;
>> + enum pkt_type type;
>> + uint32_t sa_idx;
>> + uint8_t *nlp;
>> +
>> + /* Get pkt from event */
>> + pkt = ev->mbuf;
>> +
>> + /* Check the packet type */
>> + type = process_ipsec_get_pkt_type(pkt, &nlp);
>> +
>> + switch (type) {
>> + case PKT_TYPE_PLAIN_IPV4:
>> + if (pkt->ol_flags & PKT_RX_SEC_OFFLOAD)
>> + sa = (struct ipsec_sa *) pkt->udata64;
>> +
>> + /* Check if we have a match */
>> + if (check_sp(ctx->sp4_ctx, nlp, &sa_idx) == 0) {
>> + /* No valid match */
>> + goto drop_pkt_and_exit;
>> + }
>> + break;
>> +
>> + case PKT_TYPE_PLAIN_IPV6:
>> + if (pkt->ol_flags & PKT_RX_SEC_OFFLOAD)
>> + sa = (struct ipsec_sa *) pkt->udata64;
>> +
>> + /* Check if we have a match */
>> + if (check_sp(ctx->sp6_ctx, nlp, &sa_idx) == 0) {
>> + /* No valid match */
>> + goto drop_pkt_and_exit;
>> + }
>> + break;
>> +
>> + default:
>> + RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
>> + goto drop_pkt_and_exit;
>> + }
>> +
>> + /* Check if the packet has to be bypassed */
>> + if (sa_idx == 0)
>> + goto route_and_send_pkt;
>> +
>> + /* Else the packet has to be protected with SA */
>> +
>> + /* If the packet was IPsec processed, then SA pointer should be set */
>> + if (sa == NULL)
>> + goto drop_pkt_and_exit;
>> +
>> + /* SPI on the packet should match with the one in SA */
>> + if (unlikely(sa->spi != sa_idx))
>> + goto drop_pkt_and_exit;
>> +
>> +route_and_send_pkt:
>> + port_id = get_route(pkt, rt, type);
>> + if (unlikely(port_id == RTE_MAX_ETHPORTS)) {
>> + /* no match */
>> + goto drop_pkt_and_exit;
>> + }
>> + /* else, we have a matching route */
>> +
>> + /* Update mac addresses */
>> + update_mac_addrs(pkt, port_id);
>> +
>> + /* Update the event with the dest port */
>> + ipsec_event_pre_forward(pkt, port_id);
>> + return 1;
>> +
>> +drop_pkt_and_exit:
>> + RTE_LOG(ERR, IPSEC, "Inbound packet dropped\n");
>> + rte_pktmbuf_free(pkt);
>> + ev->mbuf = NULL;
>> + return 0;
>> +}
>> +
>> /*
>> * Event mode exposes various operating modes depending on the
>> * capabilities of the event device and the operating mode
>> @@ -134,11 +345,11 @@ static void
>> ipsec_wrkr_non_burst_int_port_app_mode_inb(struct eh_event_link_info *links,
>> uint8_t nb_links)
>> {
>> + struct lcore_conf_ev_tx_int_port_wrkr lconf;
>> unsigned int nb_rx = 0;
>> - unsigned int port_id;
>> - struct rte_mbuf *pkt;
>> struct rte_event ev;
>> uint32_t lcore_id;
>> + int32_t socket_id;
>>
>> /* Check if we have links registered for this lcore */
>> if (nb_links == 0) {
>> @@ -151,6 +362,21 @@ ipsec_wrkr_non_burst_int_port_app_mode_inb(struct
>> eh_event_link_info *links,
>> /* Get core ID */
>> lcore_id = rte_lcore_id();
>>
>> + /* Get socket ID */
>> + socket_id = rte_lcore_to_socket_id(lcore_id);
>> +
>> + /* Save routing table */
>> + lconf.rt.rt4_ctx = socket_ctx[socket_id].rt_ip4;
>> + lconf.rt.rt6_ctx = socket_ctx[socket_id].rt_ip6;
>> + lconf.inbound.sp4_ctx = socket_ctx[socket_id].sp_ip4_in;
>> + lconf.inbound.sp6_ctx = socket_ctx[socket_id].sp_ip6_in;
>> + lconf.inbound.sa_ctx = socket_ctx[socket_id].sa_in;
>> + lconf.inbound.session_pool = socket_ctx[socket_id].session_pool;
>> + lconf.outbound.sp4_ctx = socket_ctx[socket_id].sp_ip4_out;
>> + lconf.outbound.sp6_ctx = socket_ctx[socket_id].sp_ip6_out;
>> + lconf.outbound.sa_ctx = socket_ctx[socket_id].sa_out;
>> + lconf.outbound.session_pool = socket_ctx[socket_id].session_pool;
>> +
>> RTE_LOG(INFO, IPSEC,
>> "Launching event mode worker (non-burst - Tx internal port - "
>> "app mode - inbound) on lcore %d\n", lcore_id);
>> @@ -175,13 +401,11 @@ ipsec_wrkr_non_burst_int_port_app_mode_inb(struct
>> eh_event_link_info *links,
>> if (nb_rx == 0)
>> continue;
>>
>> - port_id = ev.queue_id;
>> - pkt = ev.mbuf;
>> -
>> - rte_prefetch0(rte_pktmbuf_mtod(pkt, void *));
>> -
>> - /* Process packet */
>> - ipsec_event_pre_forward(pkt, port_id);
>> + if (process_ipsec_ev_inbound(&lconf.inbound,
>> + &lconf.rt, &ev) != 1) {
>> + /* The pkt has been dropped */
>> + continue;
>> + }
>>
>> /*
>> * Since tx internal port is available, events can be
>> diff --git a/examples/ipsec-secgw/ipsec_worker.h
>> b/examples/ipsec-secgw/ipsec_worker.h
>> new file mode 100644
>> index 0000000..fd18a2e
>> --- /dev/null
>> +++ b/examples/ipsec-secgw/ipsec_worker.h
>> @@ -0,0 +1,39 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2018 Cavium, Inc
>> + */
>> +#ifndef _IPSEC_WORKER_H_
>> +#define _IPSEC_WORKER_H_
>> +
>> +#include "ipsec.h"
>> +
>> +enum pkt_type {
>> + PKT_TYPE_PLAIN_IPV4 = 1,
>> + PKT_TYPE_IPSEC_IPV4,
>> + PKT_TYPE_PLAIN_IPV6,
>> + PKT_TYPE_IPSEC_IPV6,
>> + PKT_TYPE_INVALID
>> +};
>> +
>> +struct route_table {
>> + struct rt_ctx *rt4_ctx;
>> + struct rt_ctx *rt6_ctx;
>> +};
>> +
>> +/*
>> + * Conf required by event mode worker with tx internal port
>> + */
>> +struct lcore_conf_ev_tx_int_port_wrkr {
>> + struct ipsec_ctx inbound;
>> + struct ipsec_ctx outbound;
>> + struct route_table rt;
>> +} __rte_cache_aligned;
>> +
>> +/* TODO
>> + *
>> + * Move this function to ipsec_worker.c
>> + */
>> +void ipsec_poll_mode_worker(void);
>> +
>> +int ipsec_launch_one_lcore(void *args);
>> +
>> +#endif /* _IPSEC_WORKER_H_ */
>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>> index 7f046e3..9e17ba0 100644
>> --- a/examples/ipsec-secgw/sa.c
>> +++ b/examples/ipsec-secgw/sa.c
>> @@ -772,17 +772,6 @@ print_one_sa_rule(const struct ipsec_sa *sa, int
>> inbound)
>> printf("\n");
>> }
>>
>> -struct sa_ctx {
>> - void *satbl; /* pointer to array of rte_ipsec_sa objects*/
>> - struct ipsec_sa sa[IPSEC_SA_MAX_ENTRIES];
>> - union {
>> - struct {
>> - struct rte_crypto_sym_xform a;
>> - struct rte_crypto_sym_xform b;
>> - };
>> - } xf[IPSEC_SA_MAX_ENTRIES];
>> -};
>> -
>> static struct sa_ctx *
>> sa_create(const char *name, int32_t socket_id)
>> {
>> --
>> 2.7.4
>
