> -----Original Message----- > From: ext Bala Manoharan [mailto:[email protected]] > Sent: Wednesday, April 29, 2015 11:12 AM > To: Savolainen, Petri (Nokia - FI/Espoo) > Cc: ext Zoltan Kiss; [email protected] > Subject: Re: [lng-odp] [PATCH API-NEXT 4/5] api: packet_io: added parse > mode > > On 29 April 2015 at 13:24, Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> wrote: > > Hi, > > > > Possibly. Although, grouping adds struct complexity, so there should be > some benefit on doing that. I was going to add union "all" there, but > decided to leave it out since unnamed structs/unions are not defined in > C99, so the both the union and the bit field struct inside would have to > be named (for strict C99 compatibility). I think API needs to be strictly > C99, other code (linux-generic and tests) can relax a bit from that > requirement. > > > > Instead of unions, I'd defined a set of (inline) functions: > > > > // Check if all L2 flags are set > > // @return non-zero when all L2 level flags are set > > int odp_packet_parse_all_l2(const odp_packet_parse_flags_t *flags); > > One minor query on the above function, > So when application calls the above function what will be the value of > "flags"? > will it have only l2 flags set or will have other layer flags also set? >
The flags parameter has N different flags set. This function returns non-zero, if all "L2" flags are set. Other flags are ignored. > > > > // Set all L2 flags > > void odp_packet_parse_all_l2_set(odp_packet_parse_flags_t *flags); This sets all "L2" flags and does not touch other flags. -Petri > > > > ... > > > > > > Also I think the implementation would not only check levels, but for > specific flags - especially those ones it does not have HW support. E.g. > an implementation may have everything else filled in by HW, but "sctp" or > "tcp options" flags. If the application does not ask for those, the > implementation is good to go as-is. If the application asks those, the > implementation has to choose a strategy to fill in those one way or > another. > > > > > > -Petri > > > >> -----Original Message----- > >> From: ext Bala Manoharan [mailto:[email protected]] > >> Sent: Wednesday, April 29, 2015 10:25 AM > >> To: Savolainen, Petri (Nokia - FI/Espoo) > >> Cc: ext Zoltan Kiss; [email protected] > >> Subject: Re: [lng-odp] [PATCH API-NEXT 4/5] api: packet_io: added parse > >> mode > >> > >> Hi, > >> > >> Sorry I clicked send way too early in the previous mail :) > >> > >> We can optimize the odp_packet_parse_flags_t struct to support > >> both layered parsing and individual parsing by the following way. > >> > >> I believe this might be useful for both application and implementation. > >> > >> typedef struct odp_packet_parse_flags_t { > >> union { > >> uint8_t l2_all; > >> struct { > >> uint8_t eth:1; /**< See odp_packet_has_eth() > */ > >> uint8_t jumbo:1; /**< See odp_packet_has_jumbo() > */ > >> uint8_t vlan:1; /**< See odp_packet_has_vlan() > */ > >> uint8_t vlan_qinq:1; /**< See > >> odp_packet_has_vlan_qinq() */ > >> }; > >> }; > >> union { > >> uint8_t l2_all; > >> struct { > >> uint8_t arp:1; /**< See odp_packet_has_arp() */ > >> uint8_t ipv4:1; /**< See odp_packet_has_ipv4() */ > >> uint8_t ipv6:1; /**< See odp_packet_has_ipv6() */ > >> uint8_t ipfrag:1; /**< See odp_packet_has_ipfrag() */ > >> uint8_t ipopt:1; /**< See odp_packet_has_ipopt() */ > >> uint8_t ipsec:1; /**< See odp_packet_has_ipsec() */ > >> }; > >> }; > >> union { > >> uint8_t l4_all; > >> struct { > >> uint8_t udp:1; /**< See odp_packet_has_udp() */ > >> uint8_t tcp:1; /**< See odp_packet_has_tcp() */ > >> uint8_t sctp:1; /**< See odp_packet_has_sctp() */ > >> uint8_t icmp:1; /**< See odp_packet_has_icmp() */ > >> }; > >> }; > >> } odp_packet_parse_flags_t; > >> > >> Regards, > >> Bala > >> > >> On 29 April 2015 at 12:46, Bala Manoharan <[email protected]> > >> wrote: > >> > Hi, > >> > > >> > We can optimize the odp_packet_parse_flags_t in the following way to > >> > handle the layered approach for parsing > >> > > >> > +typedef struct odp_packet_parse_flags_t { > >> > > >> > + uint32_t eth:1; /**< See odp_packet_has_eth() */ > >> > + uint32_t jumbo:1; /**< See odp_packet_has_jumbo() */ > >> > + uint32_t vlan:1; /**< See odp_packet_has_vlan() */ > >> > + uint32_t vlan_qinq:1; /**< See odp_packet_has_vlan_qinq() */ > >> > + uint32_t arp:1; /**< See odp_packet_has_arp() */ > >> > + uint32_t ipv4:1; /**< See odp_packet_has_ipv4() */ > >> > + uint32_t ipv6:1; /**< See odp_packet_has_ipv6() */ > >> > + uint32_t ipfrag:1; /**< See odp_packet_has_ipfrag() */ > >> > + uint32_t ipopt:1; /**< See odp_packet_has_ipopt() */ > >> > + uint32_t ipsec:1; /**< See odp_packet_has_ipsec() */ > >> > + uint32_t udp:1; /**< See odp_packet_has_udp() */ > >> > + uint32_t tcp:1; /**< See odp_packet_has_tcp() */ > >> > + uint32_t sctp:1; /**< See odp_packet_has_sctp() */ > >> > + uint32_t icmp:1; /**< See odp_packet_has_icmp() */ > >> > + > >> > + uint32_t _reserved1:18; /**< Reserved. Do not use. */ > >> > +} odp_packet_parse_flags_t; > >> > > >> > On 29 April 2015 at 12:15, Savolainen, Petri (Nokia - FI/Espoo) > >> > <[email protected]> wrote: > >> >> It's (v2) on the list (since last Thu): > >> >> > >> >> [lng-odp] [API-NEXT PATCH v2 4/5] api: packet_io: added parse mode > >> >> > >> >> > >> >> -Petri > >> >> > >> >> > >> >>> -----Original Message----- > >> >>> From: ext Zoltan Kiss [mailto:[email protected]] > >> >>> Sent: Tuesday, April 28, 2015 9:17 PM > >> >>> To: Savolainen, Petri (Nokia - FI/Espoo); [email protected] > >> >>> Subject: Re: [lng-odp] [PATCH API-NEXT 4/5] api: packet_io: added > >> parse > >> >>> mode > >> >>> > >> >>> > >> >>> > >> >>> On 28/04/15 08:09, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >> >>> > Hi Zoltan, > >> >>> > > >> >>> > You should implement the latest version of the patch, which has > only > >> >>> ALL/NONE defined. We can leave SELECTED for later. > >> >>> Ok, but where is that version? I could only find this one. > >> >>> > >> >>> > > >> >>> > Briefly about SELECTED. The idea is that the application lists > all > >> >>> odp_packet_has_xxx() calls that it will call during packet > processing. > >> >>> Implementation can use that information to optimize parser > >> functionality, > >> >>> if it can. So, application is not telling to implementation what to > do > >> or > >> >>> how to do it, but what information application is expecting from > >> packets. > >> >>> If application lies (indicates that it will not call xxx, but still > >> calls > >> >>> it), results are undefined. > >> >>> > > >> >>> > -Petri > >> >>> > > >> >>> > > >> >>> >> -----Original Message----- > >> >>> >> From: ext Zoltan Kiss [mailto:[email protected]] > >> >>> >> Sent: Monday, April 27, 2015 8:29 PM > >> >>> >> To: Savolainen, Petri (Nokia - FI/Espoo); lng- > [email protected] > >> >>> >> Subject: Re: [lng-odp] [PATCH API-NEXT 4/5] api: packet_io: > added > >> parse > >> >>> >> mode > >> >>> >> > >> >>> >> > >> >>> >> > >> >>> >> On 20/04/15 13:10, Petri Savolainen wrote: > >> >>> >>> Application can indicate which packet parsing results it is > >> >>> >>> interested in (all, none or selected). > >> >>> >>> > >> >>> >>> Signed-off-by: Petri Savolainen <[email protected]> > >> >>> >>> --- > >> >>> >>> include/odp/api/packet_flags.h | 26 > ++++++++++++++++++++++++++ > >> >>> >>> include/odp/api/packet_io.h | 19 +++++++++++++++++++ > >> >>> >>> 2 files changed, 45 insertions(+) > >> >>> >>> > >> >>> >>> diff --git a/include/odp/api/packet_flags.h > >> >>> >> b/include/odp/api/packet_flags.h > >> >>> >>> index bfbcc94..9444fdc 100644 > >> >>> >>> --- a/include/odp/api/packet_flags.h > >> >>> >>> +++ b/include/odp/api/packet_flags.h > >> >>> >>> @@ -26,6 +26,32 @@ extern "C" { > >> >>> >>> * @{ > >> >>> >>> */ > >> >>> >>> > >> >>> >>> + > >> >>> >>> +/** > >> >>> >>> + * Packet input parsing flags > >> >>> >>> + * > >> >>> >>> + * Each flag represents a parser output. See parser output > >> functions > >> >>> >> for > >> >>> >>> + * details. > >> >>> >> > >> >>> >> Now that I implement this for linux-generic, I realized this is > >> >>> >> ambiguous: does disabling a lower layer's parsing means that the > >> parser > >> >>> >> stops looking into upper layers? Even if their parsing is > enabled? > >> >>> >> E.g. if (jumbo == 0 && ipv4 == 1), we won't parse the ethernet > >> header > >> >>> if > >> >>> >> it's a jumbo frame, that's fine. But do we continue to look into > >> the > >> >>> >> rest of the packet for the IPv4 header? > >> >>> >> I would say no, but it should be mentioned here explicitly. > >> >>> >> > >> >>> >>> + */ > >> >>> >>> +typedef struct odp_packet_parse_flags_t { > >> >>> >>> + uint32_t eth:1; /**< See odp_packet_has_eth() */ > >> >>> >>> + uint32_t jumbo:1; /**< See odp_packet_has_jumbo() */ > >> >>> >>> + uint32_t vlan:1; /**< See odp_packet_has_vlan() */ > >> >>> >>> + uint32_t vlan_qinq:1; /**< See odp_packet_has_vlan_qinq() */ > >> >>> >>> + uint32_t arp:1; /**< See odp_packet_has_arp() */ > >> >>> >>> + uint32_t ipv4:1; /**< See odp_packet_has_ipv4() */ > >> >>> >>> + uint32_t ipv6:1; /**< See odp_packet_has_ipv6() */ > >> >>> >>> + uint32_t ipfrag:1; /**< See odp_packet_has_ipfrag() */ > >> >>> >>> + uint32_t ipopt:1; /**< See odp_packet_has_ipopt() */ > >> >>> >>> + uint32_t ipsec:1; /**< See odp_packet_has_ipsec() */ > >> >>> >>> + uint32_t udp:1; /**< See odp_packet_has_udp() */ > >> >>> >>> + uint32_t tcp:1; /**< See odp_packet_has_tcp() */ > >> >>> >>> + uint32_t sctp:1; /**< See odp_packet_has_sctp() */ > >> >>> >>> + uint32_t icmp:1; /**< See odp_packet_has_icmp() */ > >> >>> >>> + > >> >>> >>> + uint32_t _reserved1:18; /**< Reserved. Do not use. */ > >> >>> >>> +} odp_packet_parse_flags_t; > >> >>> >>> + > >> >>> >>> /** > >> >>> >>> * Check for packet errors > >> >>> >>> * > >> >>> >>> diff --git a/include/odp/api/packet_io.h > >> b/include/odp/api/packet_io.h > >> >>> >>> index 77c207e..97f79ee 100644 > >> >>> >>> --- a/include/odp/api/packet_io.h > >> >>> >>> +++ b/include/odp/api/packet_io.h > >> >>> >>> @@ -18,6 +18,8 @@ > >> >>> >>> extern "C" { > >> >>> >>> #endif > >> >>> >>> > >> >>> >>> +#include <odp/packet_flags.h> > >> >>> >>> + > >> >>> >>> /** @defgroup odp_packet_io ODP PACKET IO > >> >>> >>> * Operations on a packet. > >> >>> >>> * @{ > >> >>> >>> @@ -58,6 +60,19 @@ enum odp_pktio_input_mode { > >> >>> >>> }; > >> >>> >>> > >> >>> >>> /** > >> >>> >>> + * Packet parsing mode > >> >>> >>> + */ > >> >>> >>> +enum odp_pktio_parse_mode { > >> >>> >>> + /** Parse all protocols */ > >> >>> >>> + ODP_PKTIN_PARSE_ALL = 0, > >> >>> >>> + /** Parsing not needed */ > >> >>> >>> + ODP_PKTIN_PARSE_NONE, > >> >>> >>> + /** Parsing can be limited to the flags set in > >> >>> >>> + odp_packet_parse_flags_t */ > >> >>> >>> + ODP_PKTIN_PARSE_SELECTED > >> >>> >>> +}; > >> >>> >>> + > >> >>> >>> +/** > >> >>> >>> * Packet IO parameters > >> >>> >>> * > >> >>> >>> * In minimum, user must select the input mode. Use 0 for > >> defaults. > >> >>> >> Initialize > >> >>> >>> @@ -66,6 +81,10 @@ enum odp_pktio_input_mode { > >> >>> >>> typedef struct odp_pktio_param_t { > >> >>> >>> /** Packet input mode */ > >> >>> >>> enum odp_pktio_input_mode in_mode; > >> >>> >>> + /** Packet parse mode */ > >> >>> >>> + enum odp_pktio_parse_mode parse_mode; > >> >>> >>> + /** Parse selection when parse_mode is > ODP_PKTIN_PARSE_SELECTED > >> */ > >> >>> >>> + odp_packet_parse_flags_t parse; > >> >>> >>> } odp_pktio_param_t; > >> >>> >>> > >> >>> >>> /** > >> >>> >>> > >> >> _______________________________________________ > >> >> lng-odp mailing list > >> >> [email protected] > >> >> https://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
