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?

>
>>
>>
>> >>> +{
>> >>> +       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