Hi Lukasz,

quoted

quoted

Is it not possible to use the existing functions for finding routes, checking

packet types and checking security policies.

quoted

It will be very difficult to manage two separate functions for same work. I

can

quoted

see that the pkt->data_offs

quoted

Are not required to be updated in the inline case, but can we split the existing

functions in two so that they can be

quoted

Called in the appropriate cases.

As you have said in the cover note as well to add lookaside protocol support.

I

quoted

also tried adding it, and it will get very

quoted

Difficult to manage separate functions for separate code paths.

[Lukasz] This was also Konstantin's comment during review of one of previous
revisions.
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

quoted

chunks and does much more -
it adjusts mbuf and packet length then it demultiplex packets into plain and

IPsec

quoted

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.

quoted

I understand your concern from the perspective of code maintenance but on

the

quoted

other hand we are concerned with performance.
The current code is not optimized to support multiple mode processing
introduced with rte_security. We can work on a common
routines once we have other modes also added, so that we can come up with

a

quoted

better solution than what we have today.

Yes that is correct, but we should split the existing functions so that the part
which is common
In both mode should stay common and we do not have duplicate code in the app.

I believe we should take care of this when we add lookaside cases. We shall
remove all duplicate
Code. Ideally it should be part of this patchset. But we can postpone it to the
lookaside case addition.

I believe the route(4/6)_pkts and route(4/6)_pkt can be made uniform quite easily.
Now you can call either send_single_pkt() or rte_event_eth_tx_adapter_enqueue() from
the caller of route4_pkts.
I don’t think this will impact the performance at all.
Instead of having 3 for loops, now there will be only 2 and nothing else is getting changed for
anybody. In fact we can reduce 1 more, if we can call send pkts from inside the route4_pkts.
I think that can also be done, but it may increase the lookup duration as there may be cache miss.
But that need to be experimented. What say??

static inline void
route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], uint32_t *hop[],
                uint8_t nb_pkts)
{
        uint32_t dst_ip;
        uint16_t i, offset;

        if (nb_pkts == 0)
                return;

        /* Need to do an LPM lookup for non-inline packets. Inline packets will
         * have port ID in the SA
         */

        for (i = 0; i < nb_pkts; i++) {
                if (!(pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD)) {
                        /* Security offload not enabled. So an LPM lookup is
                         * required to get the hop
                         */
                        offset = offsetof(struct ip, ip_dst);
                        dst_ip = *rte_pktmbuf_mtod_offset(pkts[i],
                                        uint32_t *, offset);
                        dst_ip = rte_be_to_cpu_32(dst_ip);
                        if (rte_lpm_lookup((struct rte_lpm *)rt_ctx, dst_ip, hop[i]))
                                rte_pktmbuf_free(pkts[i]);
                } else {
                        *hop[i] = get_hop_for_offload_pkt(pkts[i], 0);
                }
        }
}