On 28 March 2016 at 19:14, Bill Fischofer <bill.fischo...@linaro.org> wrote:
>
>
> On Mon, Mar 28, 2016 at 5:25 AM, Bala Manoharan <bala.manoha...@linaro.org
> > wrote:
>
>>
>>
>> Regards,
>> Bala
>>
>> On 24 March 2016 at 20:19, Petri Savolainen <petri.savolai...@nokia.com>
>> wrote:
>>
>>> Added options for selecting IP/UDP/TCP/SCTP checksum offload on
>>> packet input and output. Packets with input checksum failure are
>>> either dropped or reported with packet_has_l3/l4_error flags.
>>>
>>> Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com>
>>> ---
>>> include/odp/api/spec/packet_io.h | 66
>>> +++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 65 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/odp/api/spec/packet_io.h
>>> b/include/odp/api/spec/packet_io.h
>>> index 767a1b2..f9df7c3 100644
>>> --- a/include/odp/api/spec/packet_io.h
>>> +++ b/include/odp/api/spec/packet_io.h
>>> @@ -246,7 +246,9 @@ typedef struct odp_pktio_param_t {
>>> /**
>>> * Packet input configuration options bit field
>>> *
>>> - * Packet input configuration options listed in a bit field structure.
>>> + * Packet input configuration options listed in a bit field structure.
>>> Depending
>>> + * on configuration erroneous packets are either dropped or marked with
>>> error
>>> + * flags (see e.g. odp_packet_has_error() and
>>> odp_packet_has_l3_error()).
>>> */
>>> typedef union odp_pktin_config_opt_t {
>>> /** Option flags */
>>> @@ -257,6 +259,34 @@ typedef union odp_pktin_config_opt_t {
>>> /** Timestamp (at least) IEEE1588 / PTP packets
>>> * on packet input */
>>> uint64_t ts_ptp : 1;
>>> +
>>> + /** Check IPv4 header checksum on packet input */
>>> + uint64_t ipv4_chksum : 1;
>>> +
>>> + /** Check UDP checksum on packet input */
>>> + uint64_t udp_chksum : 1;
>>> +
>>> + /** Check TCP checksum on packet input */
>>> + uint64_t tcp_chksum : 1;
>>> +
>>> + /** Check SCTP checksum on packet input */
>>> + uint64_t sctp_chksum : 1;
>>> +
>>> + /** Drop packets with an IPv4 error on packet input */
>>> + uint64_t drop_ipv4_err : 1;
>>> +
>>> + /** Drop packets with an IPv6 error on packet input */
>>> + uint64_t drop_ipv6_err : 1;
>>> +
>>> + /** Drop packets with a UDP error on packet input */
>>> + uint64_t drop_udp_err : 1;
>>> +
>>> + /** Drop packets with a TCP error on packet input */
>>> + uint64_t drop_tcp_err : 1;
>>> +
>>> + /** Drop packets with a SCTP error on packet input */
>>> + uint64_t drop_sctp_err : 1;
>>> +
>>>
>>
>> We have an existing mechanism in classifier where the application can
>> configure an error CoS so that the packets with error are sent to
>> Error_CoS. So do we really need this mechanism at the interface level to
>> drop packets with error.
>>
>> } bit;
>>>
>>> /** All bits of the bit field structure
>>> @@ -267,6 +297,35 @@ typedef union odp_pktin_config_opt_t {
>>> } odp_pktin_config_opt_t;
>>>
>>> /**
>>> + * Packet output configuration options bit field
>>> + *
>>> + * Packet output configuration options listed in a bit field structure.
>>> + */
>>> +typedef union odp_pktout_config_opt_t {
>>> + /** Option flags */
>>> + struct {
>>> + /** Insert IPv4 header checksum on packet output */
>>> + uint64_t ipv4_chksum : 1;
>>> +
>>> + /** Insert UDP checksum on packet output */
>>> + uint64_t udp_chksum : 1;
>>> +
>>> + /** Insert TCP checksum on packet output */
>>> + uint64_t tcp_chksum : 1;
>>> +
>>> + /** Insert SCTP checksum on packet output */
>>> + uint64_t sctp_chksum : 1;
>>> +
>>> + } bit;
>>>
>>
>> IMO, since L3/L4 checksum calculation is a per packet operation it might
>> be better to add this in packet meta data so that if the application does
>> not modify the packet then there is no need to re-calculate the L3/L4
>> checksum. Hence if an application modifies a particular packet it can set
>> the checksum calculation bit on that packet and the implementation will
>> calculate and insert checksum for the specific packet before transmission.
>> By having this as a per-packet parameter there is no need to calculate
>> checksum for all packets in the interface.
>>
>
> That's a good point. But I don't view these as exclusive. It makes sense
> to enable/disable checksum offload at the interface level and then provide
> override capabilities on a per-packet basis so that only exceptional
> packets need additional attributes.
>
>
I am really not sure if we need this parameter in two places usually the
packets which require checksum recalculation are newly created packets and
those which are modified by the application and it seems logical that
whenever an application modifies the packet he just sets this checksum
update flag and implementation does the rest.
coz if we have this parameter in two places then for each packet which does
not need checksum update the implementation should always check two
parameters.
Regards,
Bala
>
>
>> +
>>> + /** All bits of the bit field structure
>>> + *
>>> + * This field can be used to set/clear all flags, or bitwise
>>> + * operations over the entire structure. */
>>> + uint64_t all_bits;
>>> +} odp_pktout_config_opt_t;
>>> +
>>> +/**
>>> * Packet IO configuration options
>>> *
>>> * Packet IO interface level configuration options. Use
>>> odp_pktio_capability()
>>> @@ -279,6 +338,11 @@ typedef struct odp_pktio_config_t {
>>> * Default value for all bits is zero. */
>>> odp_pktin_config_opt_t pktin;
>>>
>>> + /** Packet output configuration options bit field
>>> + *
>>> + * Default value for all bits is zero. */
>>> + odp_pktout_config_opt_t pktout;
>>> +
>>> } odp_pktio_config_t;
>>>
>>> /**
>>> --
>>> 2.7.2
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
