> -----Original Message-----
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Friday, March 24, 2017 5:39 PM
> To: Petri Savolainen <petri.savolai...@linaro.org>
> Cc: lng-odp-forward <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [API-NEXT PATCH v3 4/4] api: packet: add per packet
> checksum control
>
> This patch is independent of the IPsec patch series and should be a
> separate patch.
IPsec inline patch refers to this missing feature (as does current pktout
config).
/**
* Configuration options for IPSEC outbound processing
*/
typedef struct odp_ipsec_outbound_config_t {
/** Flags to control L3/L4 checksum insertion as part of outbound
* packet processing. Packet must have set with valid L3/L4 offsets.
* Checksum configuration is ignored for packets that checksum cannot
* be computed for (e.g. IPv4 fragments). Application may use a packet
* metadata flag to disable checksum insertion per packet bases.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
So, it is related and needs to be added soon. There's no harm to do that right
now.
> > /**
> > + * Force checksum insertion
> > + *
> > + * Override checksum insertion configuration per packet.
> > + *
> > + * @param pkt Packet handle
> > + * @param l3 0: do not insert L3 checksum
> > + * 1: insert L3 checksum
> > + * -1: use default L3 checksum configuration
> > + *
> > + * @param l4 0: do not insert L4 checksum
> > + * 1: insert L4 checksum
> > + * -1: use default L4 checksum configuration
> > + *
> > + * @retval 0 on success
> > + * @retval <0 on failure
> > + */
> > +int odp_packet_chksum_force(odp_packet_t pkt, int l3, int l4);
>
> Several comments.
>
> 1. Why is this not a void function, as presumably this function is
> advisory in nature? How would it fail and what would an application be
> expected to do in the event of a reported failure? Having to check
> return codes for this would seem to be adding a lot of overhead for
> little benefit.
>
> 2. "force" sounds awkward. Perhaps odp_packet_chksum_override(),
> odp_packet_chksum_insert(), or simply odp_packet_chksum() might be
> better?
>
> 3. To make this useful, shouldn't it be coupled with an
> odp_pktio_capability() extension that reports whether per-packet
> chksum overrides are supported on this interface, since checksum
> insertion on output is configured as part of the
> odp_pktout_config_opt_t for each odp_pktio_t?
I changed it to these two functions under. Void is correct, I did copy-paste
odp_packet_l4_offset_set() prototype which has a return value. Actually, those
offset_set functions should be changed to be void as well.
Per packet checksum control is mandatory. HW is typically implemented with per
packet control. As explained in commit log text, e.g. a L4 checksum must not be
overwritten during routing/forwarding - it's an end-to-end feature.
-Petri
/**
* Layer 3 checksum insertion override
*
* Override checksum insertion configuration per packet. This per packet setting
* overrides a higher level configuration for checksum insertion into a L3
* header during packet output processing.
*
* @param pkt Packet handle
* @param l3 0: do not insert L3 checksum
* 1: insert L3 checksum
*/
void odp_packet_l3_chksum_insert(odp_packet_t pkt, int l3);
/**
* Layer 4 checksum insertion override
*
* Override checksum insertion configuration per packet. This per packet setting
* overrides a higher level configuration for checksum insertion into a L4
* header during packet output processing.
*
* @param pkt Packet handle
* @param l4 0: do not insert L4 checksum
* 1: insert L4 checksum
*/
void odp_packet_l4_chksum_insert(odp_packet_t pkt, int l4);
