| Message ID | 1490699246-32395-1-git-send-email-petri.savolainen@linaro.org |
|---|---|
| State | Accepted |
| Commit | 096b11e8693a6dd7e681e8b21fb8ba46a1712e54 |
| Headers | show |
These look good, but shouldn't these be documented as advisory? When dealing with well-formed packets, it should not matter if HW always recomputes checksums on transmit. The only time you'd need to suppress checksum generation would be in diagnostic packet generators that are intentionally emitting packets with incorrect checksums for testing purposes. On Tue, Mar 28, 2017 at 6:07 AM, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Checksum insertion has pktio interface level configuration > options. Per packet override is needed for example when > L4 checksumming is enabled and application forwards packets. > Forwarded packets need to maintain original, end-to-end checksum > value. > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h > index 92c35ae..5439f23 100644 > --- a/include/odp/api/spec/packet.h > +++ b/include/odp/api/spec/packet.h > @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt); > int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset); > > /** > + * 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); > + > +/** > * Packet flow hash value > * > * Returns the hash generated from the packet header. Use > -- > 2.8.1 >
It's a mandatory feature when forwarding: "Forwarded packets need to maintain original, end-to-end checksum value.". A box in the middle must not re-compute e.g. L4 checksum. If it would do that, it could introduce an error in payload data and update the checksum accordingly. The receiving end would not notice the error, since checksum matches. > -----Original Message----- > From: Bill Fischofer [mailto:bill.fischofer@linaro.org] > Sent: Tuesday, March 28, 2017 2:24 PM > To: Petri Savolainen <petri.savolainen@linaro.org> > Cc: lng-odp-forward <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > These look good, but shouldn't these be documented as advisory? When > dealing with well-formed packets, it should not matter if HW always > recomputes checksums on transmit. The only time you'd need to suppress > checksum generation would be in diagnostic packet generators that are > intentionally emitting packets with incorrect checksums for testing > purposes. > > On Tue, Mar 28, 2017 at 6:07 AM, Petri Savolainen > <petri.savolainen@linaro.org> wrote: > > Checksum insertion has pktio interface level configuration > > options. Per packet override is needed for example when > > L4 checksumming is enabled and application forwards packets. > > Forwarded packets need to maintain original, end-to-end checksum > > value. > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > > --- > > include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/include/odp/api/spec/packet.h > b/include/odp/api/spec/packet.h > > index 92c35ae..5439f23 100644 > > --- a/include/odp/api/spec/packet.h > > +++ b/include/odp/api/spec/packet.h > > @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt); > > int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset); > > > > /** > > + * 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); > > + > > +/** > > * Packet flow hash value > > * > > * Returns the hash generated from the packet header. Use > > -- > > 2.8.1 > >
On Tue, Mar 28, 2017 at 6:39 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote: > It's a mandatory feature when forwarding: "Forwarded packets need to maintain original, end-to-end checksum value.". A box in the middle must not re-compute e.g. L4 checksum. If it would do that, it could introduce an error in payload data and update the checksum accordingly. The receiving end would not notice the error, since checksum matches. If the middle box can't be trusted not to tamper with the payload when forwarding, why would you trust it not to "cover its tracks" by recomputing checksums, or remember to call this API? End-to-end integrity is why protocols like IPsec are used. > > > >> -----Original Message----- >> From: Bill Fischofer [mailto:bill.fischofer@linaro.org] >> Sent: Tuesday, March 28, 2017 2:24 PM >> To: Petri Savolainen <petri.savolainen@linaro.org> >> Cc: lng-odp-forward <lng-odp@lists.linaro.org> >> Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet >> checksum control >> >> These look good, but shouldn't these be documented as advisory? When >> dealing with well-formed packets, it should not matter if HW always >> recomputes checksums on transmit. The only time you'd need to suppress >> checksum generation would be in diagnostic packet generators that are >> intentionally emitting packets with incorrect checksums for testing >> purposes. >> >> On Tue, Mar 28, 2017 at 6:07 AM, Petri Savolainen >> <petri.savolainen@linaro.org> wrote: >> > Checksum insertion has pktio interface level configuration >> > options. Per packet override is needed for example when >> > L4 checksumming is enabled and application forwards packets. >> > Forwarded packets need to maintain original, end-to-end checksum >> > value. >> > >> > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> >> > --- >> > include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++ >> > 1 file changed, 26 insertions(+) >> > >> > diff --git a/include/odp/api/spec/packet.h >> b/include/odp/api/spec/packet.h >> > index 92c35ae..5439f23 100644 >> > --- a/include/odp/api/spec/packet.h >> > +++ b/include/odp/api/spec/packet.h >> > @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt); >> > int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset); >> > >> > /** >> > + * 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); >> > + >> > +/** >> > * Packet flow hash value >> > * >> > * Returns the hash generated from the packet header. Use >> > -- >> > 2.8.1 >> >
> -----Original Message----- > From: Bill Fischofer [mailto:bill.fischofer@linaro.org] > Sent: Tuesday, March 28, 2017 3:05 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > labs.com> > Cc: lng-odp-forward <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > On Tue, Mar 28, 2017 at 6:39 AM, Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia-bell-labs.com> wrote: > > It's a mandatory feature when forwarding: "Forwarded packets need to > maintain original, end-to-end checksum value.". A box in the middle must > not re-compute e.g. L4 checksum. If it would do that, it could introduce > an error in payload data and update the checksum accordingly. The > receiving end would not notice the error, since checksum matches. > > If the middle box can't be trusted not to tamper with the payload when > forwarding, why would you trust it not to "cover its tracks" by > recomputing checksums, or remember to call this API? End-to-end > integrity is why protocols like IPsec are used. > L4 checksums are end-to-end, not per link. L4 checksum protects e.g. against memory errors in mid boxes (switches and routers). What would be the point of having a per link L4 checksum? Link layer has CRC/checksum per link already. Is this use case clear now ? -Petri
I have few questions for better understanding of usage: if check sum is 0 it will be updated or left as zero. if check sum is some wrong value (i.e. packet was modified), what is expected? if check sum is valid and this feature supported only in software, what is expected? Maxim. On 28 March 2017 at 15:28, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@nokia-bell-labs.com> wrote: > > > > -----Original Message----- > > From: Bill Fischofer [mailto:bill.fischofer@linaro.org] > > Sent: Tuesday, March 28, 2017 3:05 PM > > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > > labs.com> > > Cc: lng-odp-forward <lng-odp@lists.linaro.org> > > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > > checksum control > > > > On Tue, Mar 28, 2017 at 6:39 AM, Savolainen, Petri (Nokia - FI/Espoo) > > <petri.savolainen@nokia-bell-labs.com> wrote: > > > It's a mandatory feature when forwarding: "Forwarded packets need to > > maintain original, end-to-end checksum value.". A box in the middle must > > not re-compute e.g. L4 checksum. If it would do that, it could introduce > > an error in payload data and update the checksum accordingly. The > > receiving end would not notice the error, since checksum matches. > > > > If the middle box can't be trusted not to tamper with the payload when > > forwarding, why would you trust it not to "cover its tracks" by > > recomputing checksums, or remember to call this API? End-to-end > > integrity is why protocols like IPsec are used. > > > > L4 checksums are end-to-end, not per link. L4 checksum protects e.g. > against memory errors in mid boxes (switches and routers). What would be > the point of having a per link L4 checksum? Link layer has CRC/checksum per > link already. Is this use case clear now ? > > -Petri >
From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org]
Sent: Wednesday, March 29, 2017 10:26 AM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>
Cc: Bill Fischofer <bill.fischofer@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet checksum control
I have few questions for better understanding of usage:
if check sum is 0 it will be updated or left as zero.
if check sum is some wrong value (i.e. packet was modified), what is expected?
if check sum is valid and this feature supported only in software, what is expected?
Maxim.
Current value of the checksum field does not matter. This API (and the per interface config option) controls only checksum insertion. When enabled, ODP (HW) calculates checksum (before sending packet out of the interface) and overwrites the checksum field with the new value. When disabled, ODP does not overwrite the field.
-Petri
On 03/29/17 13:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > Sent: Wednesday, March 29, 2017 10:26 AM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> > Cc: Bill Fischofer <bill.fischofer@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet checksum control > > I have few questions for better understanding of usage: > > if check sum is 0 it will be updated or left as zero. > if check sum is some wrong value (i.e. packet was modified), what is expected? > if check sum is valid and this feature supported only in software, what is expected? > Maxim. > > > Current value of the checksum field does not matter. This API (and the per interface config option) controls only checksum insertion. When enabled, ODP (HW) calculates checksum (before sending packet out of the interface) and overwrites the checksum field with the new value. When disabled, ODP does not overwrite the field. > > -Petri > I asked about that because of naming is a little bit confusing. How about define it as: void odp_packet_out_l2_csum_update(odp_packet_t pkt, odp_bool_t enable); void odp_packet_out_l3_csum_update(odp_packet_t pkt, odp_bool_t enable); Maxim.
> -----Original Message----- > From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > Sent: Wednesday, March 29, 2017 4:16 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > labs.com> > Cc: lng-odp-forward <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > On 03/29/17 13:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > > From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > > Sent: Wednesday, March 29, 2017 10:26 AM > > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > labs.com> > > Cc: Bill Fischofer <bill.fischofer@linaro.org>; lng-odp-forward <lng- > odp@lists.linaro.org> > > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > > > I have few questions for better understanding of usage: > > > > if check sum is 0 it will be updated or left as zero. > > if check sum is some wrong value (i.e. packet was modified), what is > expected? > > if check sum is valid and this feature supported only in software, what > is expected? > > Maxim. > > > > > > Current value of the checksum field does not matter. This API (and the > per interface config option) controls only checksum insertion. When > enabled, ODP (HW) calculates checksum (before sending packet out of the > interface) and overwrites the checksum field with the new value. When > disabled, ODP does not overwrite the field. > > > > -Petri > > > > I asked about that because of naming is a little bit confusing. How > about define it as: > > void odp_packet_out_l2_csum_update(odp_packet_t pkt, odp_bool_t enable); > void odp_packet_out_l3_csum_update(odp_packet_t pkt, odp_bool_t enable); > > Maxim. Checksum update to me is: udp.chksum++; ... and insert is: udp.chksum = udp_chksum(pkt); -Petri
What about: void odp_packet_out_l3_csum_override(odp_packet_t pkt, odp_bool_t compute); On 29 March 2017 at 16:16, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/29/17 13:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >> >> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] >> Sent: Wednesday, March 29, 2017 10:26 AM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> >> Cc: Bill Fischofer <bill.fischofer@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org> >> Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet checksum control >> >> I have few questions for better understanding of usage: >> >> if check sum is 0 it will be updated or left as zero. >> if check sum is some wrong value (i.e. packet was modified), what is expected? >> if check sum is valid and this feature supported only in software, what is expected? >> Maxim. >> >> >> Current value of the checksum field does not matter. This API (and the per interface config option) controls only checksum insertion. When enabled, ODP (HW) calculates checksum (before sending packet out of the interface) and overwrites the checksum field with the new value. When disabled, ODP does not overwrite the field. >> >> -Petri >> > > I asked about that because of naming is a little bit confusing. How > about define it as: > > void odp_packet_out_l2_csum_update(odp_packet_t pkt, odp_bool_t enable); > void odp_packet_out_l3_csum_update(odp_packet_t pkt, odp_bool_t enable); > > Maxim.
On Wed, Mar 29, 2017 at 8:38 AM, Bogdan Pricope <bogdan.pricope@linaro.org> wrote: > > What about: > > void odp_packet_out_l3_csum_override(odp_packet_t pkt, odp_bool_t compute); That's a bit of a mouthful. Since chksum insertion only makes sense on output that's implied here, so I think Petri's name choices seem clear and natural. > > On 29 March 2017 at 16:16, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > On 03/29/17 13:10, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >> > >> > >> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > >> Sent: Wednesday, March 29, 2017 10:26 AM > >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> > >> Cc: Bill Fischofer <bill.fischofer@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org> > >> Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet checksum control > >> > >> I have few questions for better understanding of usage: > >> > >> if check sum is 0 it will be updated or left as zero. > >> if check sum is some wrong value (i.e. packet was modified), what is expected? > >> if check sum is valid and this feature supported only in software, what is expected? > >> Maxim. > >> > >> > >> Current value of the checksum field does not matter. This API (and the per interface config option) controls only checksum insertion. When enabled, ODP (HW) calculates checksum (before sending packet out of the interface) and overwrites the checksum field with the new value. When disabled, ODP does not overwrite the field. > >> > >> -Petri > >> > > > > I asked about that because of naming is a little bit confusing. How > > about define it as: > > > > void odp_packet_out_l2_csum_update(odp_packet_t pkt, odp_bool_t enable); > > void odp_packet_out_l3_csum_update(odp_packet_t pkt, odp_bool_t enable); > > > > Maxim.
> -----Original Message----- > From: Bill Fischofer [mailto:bill.fischofer@linaro.org] > Sent: Wednesday, March 29, 2017 6:01 PM > To: Bogdan Pricope <bogdan.pricope@linaro.org> > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>; Savolainen, Petri (Nokia - > FI/Espoo) <petri.savolainen@nokia-bell-labs.com>; lng-odp-forward <lng- > odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > On Wed, Mar 29, 2017 at 8:38 AM, Bogdan Pricope > <bogdan.pricope@linaro.org> wrote: > > > > What about: > > > > void odp_packet_out_l3_csum_override(odp_packet_t pkt, odp_bool_t > compute); > > That's a bit of a mouthful. Since chksum insertion only makes sense on > output that's implied here, so I think Petri's name choices seem clear > and natural. Also the same function will used to control IPSEC inner packet checksum insertion (and other potential tunnel offloads). So, it's not only for packet out, but generally control between "insert" vs "do not insert" checksum. -Petri
Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> On 28 March 2017 at 16:37, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Checksum insertion has pktio interface level configuration > options. Per packet override is needed for example when > L4 checksumming is enabled and application forwards packets. > Forwarded packets need to maintain original, end-to-end checksum > value. > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h > index 92c35ae..5439f23 100644 > --- a/include/odp/api/spec/packet.h > +++ b/include/odp/api/spec/packet.h > @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt); > int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset); > > /** > + * 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); > + > +/** > * Packet flow hash value > * > * Returns the hash generated from the packet header. Use > -- > 2.8.1 >
Hi,
What is the status of this patch?
Can we assume this logic?
HW Supported & configured & override insert
=> HW calculation
HW Supported & configured & override NOT insert =>
NO HW calculation
HW Supported & NOT configured & override insert =>
HW calculation
HW Supported & NOT configured & override NOT insert => NO
HW calculation
HW Not Supported
=> NO HW calculation
On 3 April 2017 at 15:43, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
>
>
> On 28 March 2017 at 16:37, Petri Savolainen <petri.savolainen@linaro.org> wrote:
>> Checksum insertion has pktio interface level configuration
>> options. Per packet override is needed for example when
>> L4 checksumming is enabled and application forwards packets.
>> Forwarded packets need to maintain original, end-to-end checksum
>> value.
>>
>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
>> ---
>> include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
>> index 92c35ae..5439f23 100644
>> --- a/include/odp/api/spec/packet.h
>> +++ b/include/odp/api/spec/packet.h
>> @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
>> int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
>>
>> /**
>> + * 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);
>> +
>> +/**
>> * Packet flow hash value
>> *
>> * Returns the hash generated from the packet header. Use
>> --
>> 2.8.1
>>
That patch is in the api-next branch now.
The checksum insertion override has basically three different states:
1) Override not set, i.e. odp_packet_l{3,4}_chksum_insert() not called for the packet
2) Override set: do not insert checksum
3) Override set: insert checksum
If the override is set, then checksum insertion should be according to it
regardless of the pktio checksum config setting, and if it is not set,
then the pktio config setting should be followed for the packet.
What is not clear is what happens if the override is set to "insert" when
the pktio capabilities indicate that the pktio checksum config flag cannot
be set. I suppose the behavior needs to be one of the following:
1) undefined behavior (the capa indicates that ODP cannot ever insert checksum)
2) checksum is not inserted but things work
3) checksum is inserted by ODP (the capability indicates just that the pktio
checksum config setting cannot be set)
The current wording about pktio capabilities in the API, IMHO, implies 3,
but maybe that is not the desired behavior.
Janne
Bogdan Pricope wrote:
>
> Hi,
>
> What is the status of this patch?
>
> Can we assume this logic?
>
> HW Supported & configured & override insert
> => HW calculation
> HW Supported & configured & override NOT insert =>
> NO HW calculation
> HW Supported & NOT configured & override insert =>
> HW calculation
> HW Supported & NOT configured & override NOT insert => NO
> HW calculation
> HW Not Supported
> => NO HW calculation
>
> On 3 April 2017 at 15:43, Bala Manoharan <bala.manoharan@linaro.org> wrote:
> > Reviewed-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>
> >
> >
> > On 28 March 2017 at 16:37, Petri Savolainen <petri.savolainen@linaro.org> wrote:
> >> Checksum insertion has pktio interface level configuration
> >> options. Per packet override is needed for example when
> >> L4 checksumming is enabled and application forwards packets.
> >> Forwarded packets need to maintain original, end-to-end checksum
> >> value.
> >>
> >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> >> ---
> >> include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++
> >> 1 file changed, 26 insertions(+)
> >>
> >> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
> >> index 92c35ae..5439f23 100644
> >> --- a/include/odp/api/spec/packet.h
> >> +++ b/include/odp/api/spec/packet.h
> >> @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
> >> int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
> >>
> >> /**
> >> + * 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);
> >> +
> >> +/**
> >> * Packet flow hash value
> >> *
> >> * Returns the hash generated from the packet header. Use
> >> --
> >> 2.8.1
> >>
Hi Janne, There is no undefined behavior: if HW csum is NOT calculated (not supported by HW OR not configured by application OR override “NOT insert”) then packet will not be modified (packet will be sent with the value set by the application). “HW Supported” = what dpdk reports as HW capabilities (odp_pktio_capability()) “Configured” = what is configured by application with odp_pktio_config(). This is a subset of what is “HW Supported”. Behavior: 1. Override not set (default) => depends on „Configured” 2. Override set: do not insert checksum => HW csum NOT calculated 3. Override set: insert checksum => depends on “HW Supported” BR, Bogdan
Hi Bogdan, I am discussing the ODP API, not a particular implementation. > if HW csum is [...] not configured by application [...] > then packet will not be modified Having checksum disabled in config does not mean the checksum will not be inserted in the packet (in api-next). In means that only if the override is not set. If override is set to "insert", then the config setting does not matter (I think this is clear in the API). Whether the capability matters is more debatable, but see the point blelow. > if HW csum is NOT calculated (not supported by HW [...] > then packet will not be modified The API says "Use odp_pktio_capability() to see which options are supported by the implementation.". So if the checksum config option is not supported, it quite obviously should not be set by the application. But that wording does not make it clear whether requesting checksum offload through override is still allowed. The API should be made more clear about it. The possible alternatives of how the behavior of the override("insert") could be defined in the API in case the checksum capa is zero include undefined behavior (i.e. application must never call override("insert") in that case), not modifying the packet (but calling override("insert") would be ok, and actually doing the insert (which, IMHO, is implied by the current text in the API spec even if not intended that way). > 3. Override set: insert checksum => depends on “HW Supported” That is just one possible way to define the behavior. Now it is left ambiguous in the API. Janne > -----Original Message----- > From: Bogdan Pricope [mailto:bogdan.pricope@linaro.org] > Sent: Friday, May 19, 2017 11:07 AM > To: Peltonen, Janne (Nokia - FI/Espoo) <janne.peltonen@nokia.com> > Cc: Bala Manoharan <bala.manoharan@linaro.org>; Petri Savolainen > <petri.savolainen@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet checksum control > > Hi Janne, > > There is no undefined behavior: if HW csum is NOT calculated (not > supported by HW OR not configured by application OR override “NOT > insert”) then packet will not be modified (packet will be sent with > the value set by the application). > > “HW Supported” = what dpdk reports as HW capabilities (odp_pktio_capability()) > “Configured” = what is configured by application with > odp_pktio_config(). This is a subset of what is “HW Supported”. > > > Behavior: > > 1. Override not set (default) => depends on „Configured” > > 2. Override set: do not insert checksum => HW csum NOT calculated > > 3. Override set: insert checksum => depends on “HW Supported” > > BR, > Bogdan
On Fri, May 19, 2017 at 3:52 AM, Peltonen, Janne (Nokia - FI/Espoo) < janne.peltonen@nokia.com> wrote: > Hi Bogdan, > > I am discussing the ODP API, not a particular implementation. > > > if HW csum is [...] not configured by application [...] > > then packet will not be modified > > Having checksum disabled in config does not mean the checksum will not > be inserted in the packet (in api-next). In means that only if the > override is not set. If override is set to "insert", then the config > setting does not matter (I think this is clear in the API). Whether > the capability matters is more debatable, but see the point blelow. > > > if HW csum is NOT calculated (not supported by HW [...] > > then packet will not be modified > > The API says "Use odp_pktio_capability() to see which options are > supported by the implementation.". So if the checksum config option > is not supported, it quite obviously should not be set by the > application. But that wording does not make it clear whether requesting > checksum offload through override is still allowed. The API should > be made more clear about it. > > The possible alternatives of how the behavior of the override("insert") > could be defined in the API in case the checksum capa is zero include > undefined behavior (i.e. application must never call override("insert") > in that case), not modifying the packet (but calling override("insert") > would be ok, and actually doing the insert (which, IMHO, is implied > by the current text in the API spec even if not intended that way). > > > 3. Override set: insert checksum => depends on “HW Supported” > > That is just one possible way to define the behavior. Now it is left > ambiguous in the API. > The override bits say what should happen to modify the odp_pktout_config_opt_t options. If the pktout configuration says "don't checksum" and the per-packet bits say "do checksum", then it's up to the implementation to insert the requested checksum or else fail the odp_pktout_send() request. Since such checksum calculation can always be done in SW, it shouldn't matter whether HW is involved. Note that the odp_pktout_config_opt_t makes no reference to how checksums are performed. One could argue that these capability bits say whether checksums are HW-assisted since any implementation should be able to insert SW checksums as needed. Perhaps that's the clarification that's needed? In this case the odp_pktin_config_opt_t and odp_pktout_config_opt_t returned by odp_pktio_capability() should be clarified as referring to HW support for checksum validation/insertion. > > Janne > > > > -----Original Message----- > > From: Bogdan Pricope [mailto:bogdan.pricope@linaro.org] > > Sent: Friday, May 19, 2017 11:07 AM > > To: Peltonen, Janne (Nokia - FI/Espoo) <janne.peltonen@nokia.com> > > Cc: Bala Manoharan <bala.manoharan@linaro.org>; Petri Savolainen > > <petri.savolainen@linaro.org>; lng-odp-forward <lng-odp@lists.linaro.org > > > > Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet: add per packet > checksum control > > > > Hi Janne, > > > > There is no undefined behavior: if HW csum is NOT calculated (not > > supported by HW OR not configured by application OR override “NOT > > insert”) then packet will not be modified (packet will be sent with > > the value set by the application). > > > > “HW Supported” = what dpdk reports as HW capabilities > (odp_pktio_capability()) > > “Configured” = what is configured by application with > > odp_pktio_config(). This is a subset of what is “HW Supported”. > > > > > > Behavior: > > > > 1. Override not set (default) => depends on „Configured” > > > > 2. Override set: do not insert checksum => HW csum NOT calculated > > > > 3. Override set: insert checksum => depends on “HW Supported” > > > > BR, > > Bogdan >
Bill Fischofer wrote: > If the pktout configuration says "don't checksum" and the per-packet bits > say "do checksum", then it's up to the implementation to insert the requested > checksum or else fail the odp_pktout_send() request. The API spec says the following, implying that it is perfectly fine to request checksumming through the override and expect it to also happen at packet output even when the pktio level configuration is "don't checksum". If it were not, then value 1 for the l3 parameter would not be very useful. /** * 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); Whether the data returned through the capability query is relevant for the above is not clear in the API and obviously different people make different interpretations or assumptions now. > it shouldn't matter whether HW is involved. Note that the odp_pktout_config_opt_t > makes no reference to how checksums are performed. Yes, now it is an internal ODP implementation issue whether HW is involved in the calculation or not. > One could argue that these capability bits say whether checksums are HW-assisted > since any implementation should be able to insert SW checksums as needed. > Perhaps that's the clarification that's needed? In this case the > odp_pktin_config_opt_t and odp_pktout_config_opt_t returned by > odp_pktio_capability() should be clarified as referring to HW support > for checksum validation/insertion. I think that is also a possible way to resolve the unclarity. Maybe Petri wants to comment which way he would prefer when he comes back. Janne
diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h index 92c35ae..5439f23 100644 --- a/include/odp/api/spec/packet.h +++ b/include/odp/api/spec/packet.h @@ -1366,6 +1366,32 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt); int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset); /** + * 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); + +/** * Packet flow hash value * * Returns the hash generated from the packet header. Use
Checksum insertion has pktio interface level configuration options. Per packet override is needed for example when L4 checksumming is enabled and application forwards packets. Forwarded packets need to maintain original, end-to-end checksum value. Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- include/odp/api/spec/packet.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) -- 2.8.1