On 14 November 2014 14:50, Ciprian Barbu <[email protected]> wrote:

> On Tue, Nov 11, 2014 at 3:01 PM, Venkatesh Vivekanandan
> <[email protected]> wrote:
> >
> >
> > On 11 November 2014 17:52, Ciprian Barbu <[email protected]>
> wrote:
> >>
> >> On Tue, Nov 11, 2014 at 1:35 PM, Ciprian Barbu <
> [email protected]>
> >> wrote:
> >> > On Tue, Nov 11, 2014 at 1:29 PM, Venkatesh Vivekanandan
> >> > <[email protected]> wrote:
> >> >> Only review comment was to change the copyright of odp_packet_io.h to
> >> >> 2014
> >> >> from Anders.
> >> >>
> >> >> Since the comment is trivial, didn't send patch v2.
> >> >>
> >> >> Incorporated the changes and merged!
> >> >>
> >> >> Thanks,
> >> >> Venky
> >> >>
> >> >> On 7 November 2014 19:08, <[email protected]> wrote:
> >> >>>
> >> >>> From: Venkatesh Vivekanandan <[email protected]>
> >> >>>
> >> >>> - Enabled crypto support in ODP-DPDK by calling
> odp_crypto_init_global
> >> >>>   during initialization.
> >> >>> - Added platform specific odp_packet_io.h
> >> >>> - Added odp_pktio_get_mac_addr API for ODP-DPDK.
> >> >>> - Fixed data_len of mbuf.
> >> >>> - set USE_MAC_ADDR_HACK to 0 in odp_ipsec.c to use odp API to get
> mac
> >> >>>   addr in ODP-DPDK platform. This is a temporary fix until ipsec app
> >> >>>   is modified to use odp_pktio_get_mac_addr permanently.
> >> >>>
> >> >>> Signed-off-by: Venkatesh Vivekanandan
> >> >>> <[email protected]>
> >> >>> ---
> >> >>>  platform/linux-dpdk/Makefile.am                 |   2 +-
> >> >>>  platform/linux-dpdk/include/api/odp_packet_io.h | 152
> >> >>> ++++++++++++++++++++++++
> >> >>>  platform/linux-dpdk/odp_init.c                  |   5 +
> >> >>>  platform/linux-dpdk/odp_packet.c                |   1 +
> >> >>>  platform/linux-dpdk/odp_packet_io.c             |  12 ++
> >> >>>  5 files changed, 171 insertions(+), 1 deletion(-)
> >> >>>  create mode 100644 platform/linux-dpdk/include/api/odp_packet_io.h
> >> >>>
> >> >>> diff --git a/platform/linux-dpdk/Makefile.am
> >> >>> b/platform/linux-dpdk/Makefile.am
> >> >>> index 3d68729..686df6a 100644
> >> >>> --- a/platform/linux-dpdk/Makefile.am
> >> >>> +++ b/platform/linux-dpdk/Makefile.am
> >> >>> @@ -38,7 +38,7 @@ include_HEADERS = \
> >> >>>
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_init.h \
> >> >>>
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_packet_flags.h
> \
> >> >>>                   $(srcdir)/include/api/odp_packet.h \
> >> >>> -
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_packet_io.h \
> >> >>> +                 $(srcdir)/include/api/odp_packet_io.h \
> >> >>>
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_queue.h \
> >> >>>
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_rwlock.h \
> >> >>>
> >> >>> $(top_srcdir)/platform/linux-generic/include/api/odp_schedule.h \
> >> >>> diff --git a/platform/linux-dpdk/include/api/odp_packet_io.h
> >> >>> b/platform/linux-dpdk/include/api/odp_packet_io.h
> >> >>> new file mode 100644
> >> >>> index 0000000..0c3be54
> >> >>> --- /dev/null
> >> >>> +++ b/platform/linux-dpdk/include/api/odp_packet_io.h
> >> >>> @@ -0,0 +1,152 @@
> >> >>> +/* Copyright (c) 2013, Linaro Limited
> >> >>> + * All rights reserved.
> >> >>> + *
> >> >>> + * SPDX-License-Identifier:     BSD-3-Clause
> >> >>> + */
> >> >>> +
> >> >>> +
> >> >>> +/**
> >> >>> + * @file
> >> >>> + *
> >> >>> + * ODP Packet IO
> >> >>> + */
> >> >>> +
> >> >>> +#ifndef ODP_PACKET_IO_H_
> >> >>> +#define ODP_PACKET_IO_H_
> >> >>> +
> >> >>> +#ifdef __cplusplus
> >> >>> +extern "C" {
> >> >>> +#endif
> >> >>> +
> >> >>> +#include <odp_std_types.h>
> >> >>> +#include <odp_buffer_pool.h>
> >> >>> +#include <odp_packet.h>
> >> >>> +#include <odp_queue.h>
> >> >>> +
> >> >>> +#include <odp_pktio_types.h>
> >> >>> +
> >> >>> +/** ODP packet IO handle */
> >> >>> +typedef uint32_t odp_pktio_t;
> >> >>> +
> >> >>> +/** Invalid packet IO handle */
> >> >>> +#define ODP_PKTIO_INVALID 0
> >> >>> +
> >> >>> +/**
> >> >>> + * Open an ODP packet IO instance
> >> >>> + *
> >> >>> + * @param dev    Packet IO device
> >> >>> + * @param pool   Pool to use for packet IO
> >> >>> + * @param params Set of parameters to pass to the arch dependent
> >> >>> implementation
> >> >>> + *
> >> >>> + * @return ODP packet IO handle or ODP_PKTIO_INVALID on error
> >> >>> + */
> >> >>> +odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool,
> >> >>> +                          odp_pktio_params_t *params);
> >> >>> +
> >> >>> +/**
> >> >>> + * Close an ODP packet IO instance
> >> >>> + *
> >> >>> + * @param id  ODP packet IO handle
> >> >>> + *
> >> >>> + * @return 0 on success or -1 on error
> >> >>> + */
> >> >>> +int odp_pktio_close(odp_pktio_t id);
> >> >>> +
> >> >>> +/**
> >> >>> + * Receive packets
> >> >>> + *
> >> >>> + * @param id          ODP packet IO handle
> >> >>> + * @param pkt_table[] Storage for received packets (filled by
> >> >>> function)
> >> >>> + * @param len         Length of pkt_table[], i.e. max number of
> pkts
> >> >>> to
> >> >>> receive
> >> >>> + *
> >> >>> + * @return Number of packets received or -1 on error
> >> >>> + */
> >> >>> +int odp_pktio_recv(odp_pktio_t id, odp_packet_t pkt_table[],
> unsigned
> >> >>> len);
> >> >>> +
> >> >>> +/**
> >> >>> + * Send packets
> >> >>> + *
> >> >>> + * @param id           ODP packet IO handle
> >> >>> + * @param pkt_table[]  Array of packets to send
> >> >>> + * @param len          length of pkt_table[]
> >> >>> + *
> >> >>> + * @return Number of packets sent or -1 on error
> >> >>> + */
> >> >>> +int odp_pktio_send(odp_pktio_t id, odp_packet_t pkt_table[],
> unsigned
> >> >>> len);
> >> >>> +
> >> >>> +/**
> >> >>> + * Set the default input queue to be associated with a pktio handle
> >> >>> + *
> >> >>> + * @param id   ODP packet IO handle
> >> >>> + * @param queue default input queue set
> >> >>> + * @return  0 on success or -1 on error
> >> >>> + */
> >> >>> +int odp_pktio_inq_setdef(odp_pktio_t id, odp_queue_t queue);
> >> >>> +
> >> >>> +/**
> >> >>> + * Get default input queue associated with a pktio handle
> >> >>> + *
> >> >>> + * @param id  ODP packet IO handle
> >> >>> + *
> >> >>> + * @return Default input queue set or ODP_QUEUE_INVALID on error
> >> >>> + */
> >> >>> +odp_queue_t odp_pktio_inq_getdef(odp_pktio_t id);
> >> >>> +
> >> >>> +/**
> >> >>> + * Remove default input queue (if set)
> >> >>> + *
> >> >>> + * @param id  ODP packet IO handle
> >> >>> + *
> >> >>> + * @return 0 on success or -1 on error
> >> >>> + */
> >> >>> +int odp_pktio_inq_remdef(odp_pktio_t id);
> >> >>> +
> >> >>> +/**
> >> >>> + * Query default output queue
> >> >>> + *
> >> >>> + * @param id ODP packet IO handle
> >> >>> + *
> >> >>> + * @return Default out queue or ODP_QUEUE_INVALID on error
> >> >>> + */
> >> >>> +odp_queue_t odp_pktio_outq_getdef(odp_pktio_t id);
> >> >>> +
> >> >>> +/**
> >> >>> + * Store packet input handle into packet
> >> >>> + *
> >> >>> + * @param pkt  ODP packet buffer handle
> >> >>> + * @param id   ODP packet IO handle
> >> >>> + *
> >> >>> + * @return
> >> >>> + */
> >> >>> +void odp_pktio_set_input(odp_packet_t pkt, odp_pktio_t id);
> >> >>> +
> >> >>> +/**
> >> >>> + * Get stored packet input handle from packet
> >> >>> + *
> >> >>> + * @param pkt  ODP packet buffer handle
> >> >>> + *
> >> >>> + * @return Packet IO handle
> >> >>> + */
> >> >>> +odp_pktio_t odp_pktio_get_input(odp_packet_t pkt);
> >> >>> +
> >> >>> +/**
> >> >>> + * Defines the maximum length of mac address supported by this
> >> >>> platform
> >> >>> + */
> >> >>> +#define ODP_MAC_ADDR_MAX_LENGTH        ETH_ALEN
> >> >>> +
> >> >>> +/**
> >> >>> + * Get mac address of the interface
> >> >>> + *
> >> >>> + * @param id           ODP packet IO handle
> >> >>> + * @param mac_addr     Storage for Mac address of the packet IO
> >> >>> interface
> >> >>> + *                     Storage provided by the caller should be
> equal
> >> >>> + *                     to ODP_MAC_ADDR_MAX_LENGTH (filled by
> >> >>> function)
> >> >>> + * @return  0 on success or -1 on error
> >> >>> +**/
> >> >>> +int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char
> *mac_addr);
> >> >>> +
> >> >>> +#ifdef __cplusplus
> >> >>> +}
> >> >>> +#endif
> >> >>> +
> >> >>> +#endif
> >> >>> diff --git a/platform/linux-dpdk/odp_init.c
> >> >>> b/platform/linux-dpdk/odp_init.c
> >> >>> index ecc2066..404e616 100644
> >> >>> --- a/platform/linux-dpdk/odp_init.c
> >> >>> +++ b/platform/linux-dpdk/odp_init.c
> >> >>> @@ -91,6 +91,11 @@ int odp_init_global(void)
> >> >>>                 return -1;
> >> >>>         }
> >> >>>
> >> >>> +       if (odp_crypto_init_global()) {
> >> >>> +               ODP_ERR("ODP crypto init failed.\n");
> >> >>> +               return -1;
> >> >>> +       }
> >> >>> +
> >> >>>         return 0;
> >> >>>  }
> >> >>>
> >> >>> diff --git a/platform/linux-dpdk/odp_packet.c
> >> >>> b/platform/linux-dpdk/odp_packet.c
> >> >>> index 1cd190d..7b6b082 100644
> >> >>> --- a/platform/linux-dpdk/odp_packet.c
> >> >>> +++ b/platform/linux-dpdk/odp_packet.c
> >> >>> @@ -74,6 +74,7 @@ static int odp_packet_set_offset_len(odp_packet_t
> >> >>> pkt,
> >> >>> size_t frame_offset,
> >> >>>                 return -1;
> >> >>>         }
> >> >>>         mb->pkt.pkt_len = len;
> >> >>> +       mb->pkt.data_len = len;
> >> >
> >> > I think this change is not entirely correct, data_len is related to
> >> > segmentation, it can't always be equal to pkt_len. Give me a minute to
> >> > check it out.
> >>
> >> Ok, so mb->pkt.data_len is the length of the segment represented by
> >> this rte_mbuf. The other one, mb->pkt.pkt_len is the total length of
> >> the packet, when the ret_mbuf in question is the first segment in a
> >> scattered packet. When I wrote odp_packet_set_offset_len I tried to
> >> implement a similar mechanism to what there is in linux-generic buffer
> >> management, where you can have a frame_offset to get proper alignment.
> >> But DPDK rte_mbufs also support headroom, so something doesn't make
> >> sense here. I think the best way forward with this issue is to update
> >> to latest packet API design, where there is no more odp_packet_set_len
> >> routine, but instead there are APIs like odp_packet_push_tail and
> >> odp_packet_pull_tail. In the short term though, you can go ahead with
> >> the change you proposed, setting pkt.data_len to len.
> >>
> >> >
> >> >>>
> >> >>>         return 0;
> >> >>>  }
> >> >>> diff --git a/platform/linux-dpdk/odp_packet_io.c
> >> >>> b/platform/linux-dpdk/odp_packet_io.c
> >> >>> index 18635de..79394bb 100644
> >> >>> --- a/platform/linux-dpdk/odp_packet_io.c
> >> >>> +++ b/platform/linux-dpdk/odp_packet_io.c
> >> >>> @@ -422,3 +422,15 @@ int pktin_deq_multi(queue_entry_t *qentry,
> >> >>> odp_buffer_hdr_t *buf_hdr[], int num)
> >> >>>
> >> >>>         return nbr;
> >> >>>  }
> >> >>> +
> >> >>> +int odp_pktio_get_mac_addr(odp_pktio_t id, unsigned char *mac_addr)
> >>
> >> The Packet I/O Design Doc refers to this function as
> >> 'odp_pktio_mac_addr', you should have a look there.
> >
> >
> > I had taken the name from ipsec app "as is" including parameters and
> didn't
> > want to change the ipsec app. Once the pktio design is frozen and linux
> > generic has this API, I will match it to that.
>
> Ok, but the ipsec app uses a "hack" to get the mac address:
>
> https://git.linaro.org/lng/odp.git/blob/HEAD:/example/ipsec/odp_ipsec.c#l587
>
> So odp_pktio_get_mac_addr is not even declared, if you're introducing
> an API that has already been described (in a draft documentation, it's
> true, but nevertheless it follows our rules of naming APIs) then why
> not do it the right way?
>

If odp_pktio_mac_addr is used in linux-dpdk, ipsec app has to be modified.
When USE_MAC_ADDR_HACK is unset, app will look for odp_pktio_get_mac_addr
which it can't find. A patch for ipsec app will be unnecessary, since we
know for sure this is going to change(name and parameters are bound to
change until it is implemented).

Bug 627 says:

The API is under discussion in pktio design doc.
Would prefer to have the doc as frozen before posting linux generic patch

>
> >>
> >>
> >> >>> +{
> >> >>> +       pktio_entry_t *pktio_entry = get_entry(id);
> >> >>> +       if (!pktio_entry) {
> >> >>> +               ODP_ERR("Invalid odp_pktio_t value\n");
> >> >>> +               return -1;
> >> >>> +       }
> >> >>> +       rte_eth_macaddr_get(pktio_entry->s.pkt_dpdk.portid,
> >> >>> +                           (struct ether_addr *)mac_addr);
> >> >>> +       return 0;
> >> >>> +}
> >> >>> --
> >> >>> 1.9.1
> >> >>>
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> lng-odp mailing list
> >> >> [email protected]
> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >> >>
> >
> >
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to