On Thu, Jan 21, 2016 at 9:30 AM, Savolainen, Petri (Nokia - FI/Espoo) < [email protected]> wrote:
> > > > > *From:* EXT Bill Fischofer [mailto:[email protected]] > *Sent:* Thursday, January 21, 2016 4:43 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* LNG ODP Mailman List > *Subject:* Re: [lng-odp] [API-NEXT PATCH] api: packet: added multicast > flags > > > > > > > > On Thu, Jan 21, 2016 at 3:39 AM, Petri Savolainen < > [email protected]> wrote: > > Added packet flags for Ethernet and IP broad- and multicast. > For application, it's more effective to check a flag than all > destionation address bits. > > Signed-off-by: Petri Savolainen <[email protected]> > --- > include/odp/api/packet_flags.h | 70 > +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 69 insertions(+), 1 deletion(-) > > diff --git a/include/odp/api/packet_flags.h > b/include/odp/api/packet_flags.h > index 8de0c82..65fd3ce 100644 > --- a/include/odp/api/packet_flags.h > +++ b/include/odp/api/packet_flags.h > @@ -102,11 +102,29 @@ int odp_packet_has_l4(odp_packet_t pkt); > * > * @param pkt Packet handle > * @retval non-zero if packet contains a valid eth header > - * @retval 0 if packet does not contain a valid & known eth header > + * @retval 0 if packet does not contain a valid eth header > */ > int odp_packet_has_eth(odp_packet_t pkt); > > /** > + * Check for Ethernet broadcast address > + * > + * @param pkt Packet handle > + * @retval non-zero if Ethernet destination address is the broadcast > address > + * @retval 0 if Ethernet destination address is not the broadcast address > + */ > +int odp_packet_has_eth_bcast(odp_packet_t pkt); > + > +/** > + * Check for Ethernet multicast address > + * > + * @param pkt Packet handle > + * @retval non-zero if Ethernet destination address is a multicast address > + * @retval 0 if Ethernet destination address is not a multicast address > + */ > +int odp_packet_has_eth_mcast(odp_packet_t pkt); > + > +/** > * Check for jumbo frame > * > * @param pkt Packet handle > @@ -161,6 +179,24 @@ int odp_packet_has_ipv4(odp_packet_t pkt); > int odp_packet_has_ipv6(odp_packet_t pkt); > > /** > + * Check for IP broadcast address > + * > + * @param pkt Packet handle > + * @retval non-zero if IP destination address is a broadcast address > + * @retval 0 if IP destination address is not a broadcast address > + */ > +int odp_packet_has_ip_bcast(odp_packet_t pkt); > + > +/** > + * Check for IP multicast address > + * > + * @param pkt Packet handle > + * @retval non-zero if IP destination address is a multicast address > + * @retval 0 if IP destination address is not a multicast address > + */ > +int odp_packet_has_ip_mcast(odp_packet_t pkt); > + > > > > Should there be an odp_packet_has_ip_anycast() API for completeness? > > > > > > I don’t remember seeing that commonly supported by HW parsers whereas the > two above are. It can be always added later if commonly needed and > supported. > > > Anycast is a mandatory part of IPv6 so I don't see why we'd want to omit it. Given the complexities of these parses, I'd imagine SW parsers would treat these as a separately-handled lazy parse case (i.e., no parse is done until one of these is actually called since the expectation would be that they'd be called rarely). > > > > > > > +/** > * Check for IP fragment > * > * @param pkt Packet handle > @@ -265,6 +301,22 @@ void odp_packet_has_l4_set(odp_packet_t pkt, int val); > void odp_packet_has_eth_set(odp_packet_t pkt, int val); > > /** > + * Set flag for Ethernet broadcast address > + * > + * @param pkt Packet handle > + * @param val Value > + */ > +void odp_packet_has_eth_bcast_set(odp_packet_t pkt, int val); > + > +/** > + * Set flag for Ethernet multicast address > + * > + * @param pkt Packet handle > + * @param val Value > + */ > +void odp_packet_has_eth_mcast_set(odp_packet_t pkt, int val); > + > +/** > * Set flag for jumbo frame > * > * @param pkt Packet handle > @@ -313,6 +365,22 @@ void odp_packet_has_ipv4_set(odp_packet_t pkt, int > val); > void odp_packet_has_ipv6_set(odp_packet_t pkt, int val); > > /** > + * Set flag for IP broadcast address > + * > + * @param pkt Packet handle > + * @param val Value > + */ > +void odp_packet_has_ip_bcast_set(odp_packet_t pkt, int val); > + > +/** > + * Set flag for IP multicast address > + * > + * @param pkt Packet handle > + * @param val Value > + */ > +void odp_packet_has_ip_mcast_set(odp_packet_t pkt, int val); > + > > > > The query APIs for these attributes are fine, however given that these are > encoded in the Ethernet/IP addresses I'm not sure that set variants make > sense here. Wouldn't these be indirectly changing the Ethernet/IP > address? Especially in the case of IPv6 these tend to follow complex rules > that aren't simple booleans. > > > > As any other flags, if user changes the packet (e.g. re-uses it for an > unicast packet) or allocates a new one, he can reset / set relevant flags. > The next module the pipeline (HW or SW) needs to see updated flags. > > > > -Petri > > > As we add more of these attributes I continue to believe allowing the packet to be modified and the parse flags to be set independently is both wasteful (user needs to make multiple calls) and error-prone. The more such "set" calls there are the more likely it is that applications will forget to make one or do so in an inconsistent manner, at which point they lose their usefulness (and trustworthiness). It would be better to have an explicit odp_packet_parse() or odp_packet_parse_reset() API that would either force a reparse of the modified packet (resulting in a guaranteed known good parse) or simply reset the parse by indicating that the previous parse is now stale and resulting in a new lazy parse as needed should these attributes be subsequently referenced. > > > > > +/** > * Set flag for IP fragment > * > * @param pkt Packet handle > -- > 2.6.3 > > _______________________________________________ > 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
