Message ID | 1493917206-2630-2-git-send-email-odpbot@yandex.ru |
---|---|
State | New |
Headers | show |
Series | [API-NEXT,v1,1/2] api: ipsec: add soft limit expiration event | expand |
> -----Original Message----- > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > Github ODP bot > Sent: Thursday, May 04, 2017 8:00 PM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits > expiration to flags, rather than errors > > From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> > > Soft limit expiration isn't an error per se. It does not mean, that we > received invalid or unprocessed packet. They look more like flags, > noting that soft limit on this SA was expired. Advantage of keeping those among error flags, is the easy/fast check in the common case: Only check (error.u64 == 0) to see that everything is OK vs. checking always both (error.u64 == 0 && (status.u64 == 0 || (status.soft_exp_sec == 0 && status.soft_exp_bytes == 0 && status.soft_exp_packets == 0))) -Petri
There is perhaps some ambiguity now in the API on whether expired soft lifetime means that a packet is not "successfully processed", which in turn determines whether inline mode outbound packets get sent directly out. Having the life time expiration in the error field could be interpreted to imply that soft life time expiration in inline output is not considered successful processing, so an application can expect getting a result event with successfully transformed packet but with one of the soft lifetime error bits set. That works, but I do not think we want to drop out from inline processing mode just because of soft lifetime (/bytes/packets) expiration. Janne > -----Original Message----- > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Savolainen, Petri > (Nokia - FI/Espoo) > Sent: Friday, May 05, 2017 11:18 AM > To: lng-odp@lists.linaro.org > Subject: Suspected SPAM - Re: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft > limits expiration to flags, rather than errors > > > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > > Github ODP bot > > Sent: Thursday, May 04, 2017 8:00 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits > > expiration to flags, rather than errors > > > > From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> > > > > Soft limit expiration isn't an error per se. It does not mean, that we > > received invalid or unprocessed packet. They look more like flags, > > noting that soft limit on this SA was expired. > > Advantage of keeping those among error flags, is the easy/fast check in the common case: > > Only check (error.u64 == 0) to see that everything is OK vs. > > checking always both (error.u64 == 0 && (status.u64 == 0 || (status.soft_exp_sec == 0 && > status.soft_exp_bytes == 0 && status.soft_exp_packets == 0))) > > > -Petri
On Fri, May 5, 2017 at 9:01 AM, Peltonen, Janne (Nokia - FI/Espoo) < janne.peltonen@nokia.com> wrote: > There is perhaps some ambiguity now in the API on whether expired soft > lifetime means that a packet is not "successfully processed", which in > turn determines whether inline mode outbound packets get sent directly out. > > Having the life time expiration in the error field could be interpreted > to imply that soft life time expiration in inline output is not considered > successful processing, so an application can expect getting a result event > with successfully transformed packet but with one of the soft lifetime > error bits set. That works, but I do not think we want to drop out from > inline processing mode just because of soft lifetime (/bytes/packets) > Agree. which is why soft limits should be treated as separate events. These are essentially alerts to the control plane that it's time to start rekeying this SA as the current keys are nearing the end of their useful life. So in that case it's like the low fuel indicator on a car. That doesn't mean the car stops, just means that you need to consider filling up in the near future. Since as previously noted time based limits need to be treated as events anyway since they are not associated with any packet, it's simpler and more uniform to treat all such limit crossings the same. Adding side-information to the processing results for a given packet means that the application would need to check every packet for these rather than simply having the appropriate event handler waiting to receive these relatively rare notifications. > expiration. > > Janne > > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > Savolainen, Petri > > (Nokia - FI/Espoo) > > Sent: Friday, May 05, 2017 11:18 AM > > To: lng-odp@lists.linaro.org > > Subject: Suspected SPAM - Re: [lng-odp] [PATCH API-NEXT v1 2/2] api: > ipsec: move soft > > limits expiration to flags, rather than errors > > > > > > > > > -----Original Message----- > > > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > > > Github ODP bot > > > Sent: Thursday, May 04, 2017 8:00 PM > > > To: lng-odp@lists.linaro.org > > > Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits > > > expiration to flags, rather than errors > > > > > > From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> > > > > > > Soft limit expiration isn't an error per se. It does not mean, that we > > > received invalid or unprocessed packet. They look more like flags, > > > noting that soft limit on this SA was expired. > > > > Advantage of keeping those among error flags, is the easy/fast check in > the common case: > > > > Only check (error.u64 == 0) to see that everything is OK vs. > > > > checking always both (error.u64 == 0 && (status.u64 == 0 || > (status.soft_exp_sec == 0 && > > status.soft_exp_bytes == 0 && status.soft_exp_packets == 0))) > > > > > > -Petri > >
On Fri, May 5, 2017 at 3:17 AM, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@nokia.com> wrote: > > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of > > Github ODP bot > > Sent: Thursday, May 04, 2017 8:00 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits > > expiration to flags, rather than errors > > > > From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> > > > > Soft limit expiration isn't an error per se. It does not mean, that we > > received invalid or unprocessed packet. They look more like flags, > > noting that soft limit on this SA was expired. > > Advantage of keeping those among error flags, is the easy/fast check in > the common case: > > Only check (error.u64 == 0) to see that everything is OK vs. > > checking always both (error.u64 == 0 && (status.u64 == 0 || > (status.soft_exp_sec == 0 && status.soft_exp_bytes == 0 && > status.soft_exp_packets == 0))) > Agree on keeping error bits together for check simplicity, but as discussed on other threads status events should be their own events, not piggybacked onto operation results since these are essentially control signals rather than things that worker threads would process. The point is that a worker receiving this notification can only pass it along to someone else since workers aren't (or shouldn't) be involved in rekeying. So better to have a separate control thread that receives such notifications directly. > > > -Petri > >
On 05.05.2017 11:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of >> Github ODP bot >> Sent: Thursday, May 04, 2017 8:00 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits >> expiration to flags, rather than errors >> >> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> >> >> Soft limit expiration isn't an error per se. It does not mean, that we >> received invalid or unprocessed packet. They look more like flags, >> noting that soft limit on this SA was expired. > > Advantage of keeping those among error flags, is the easy/fast check in the common case: > > Only check (error.u64 == 0) to see that everything is OK vs. Well, that is exactly my point for moving soft_exp out of error bitfield. Soft expiration is not an error. The packet was processed successfully. In case of outbound inline processing, it was even sent to pktio. I have following code scenarios for the application (leaving alone time based expiration, which should be rethought, especially wrt. SYNC processing): - SYNC or ASYNC: application checks status.flag.soft_exp_* to check if it needs to rekey. Additional care should be applied so that rekeying is not started multiple times for the same SA, because further packet status will also contains soft_exp_* flag set on. - ASYNC or INLINE: application receives IPSEC_STATUS event for this SA queue and then starts rekeying based on that event. I should probably note that there is no direct need to introduce IPSEC_STATUS events for hard timeouts, since they will end up as errors. -- With best wishes Dmitry
On Fri, May 5, 2017 at 4:24 PM, Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> wrote: > On 05.05.2017 11:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >> >>> -----Original Message----- >>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of >>> Github ODP bot >>> Sent: Thursday, May 04, 2017 8:00 PM >>> To: lng-odp@lists.linaro.org >>> Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits >>> expiration to flags, rather than errors >>> >>> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> >>> >>> Soft limit expiration isn't an error per se. It does not mean, that we >>> received invalid or unprocessed packet. They look more like flags, >>> noting that soft limit on this SA was expired. >> >> Advantage of keeping those among error flags, is the easy/fast check in the common case: >> >> Only check (error.u64 == 0) to see that everything is OK vs. > > Well, that is exactly my point for moving soft_exp out of error > bitfield. Soft expiration is not an error. The packet was processed > successfully. In case of outbound inline processing, it was even sent to > pktio. > > I have following code scenarios for the application (leaving alone time > based expiration, which should be rethought, especially wrt. SYNC > processing): > > - SYNC or ASYNC: application checks status.flag.soft_exp_* to check if > it needs to rekey. Additional care should be applied so that rekeying is > not started multiple times for the same SA, because further packet > status will also contains soft_exp_* flag set on. > > - ASYNC or INLINE: application receives IPSEC_STATUS event for this SA > queue and then starts rekeying based on that event. > > I should probably note that there is no direct need to introduce > IPSEC_STATUS events for hard timeouts, since they will end up as errors. Hard and soft timeouts have the same problem in that they are independent of any packet. So if no packets are being processed you might never notice that a hard timeout has passed. Agree we need to discuss this next week, but treating all such expirations uniformly as async events seems consistent and straightforward. > > -- > With best wishes > Dmitry
On 06.05.2017 00:29, Bill Fischofer wrote: > On Fri, May 5, 2017 at 4:24 PM, Dmitry Eremin-Solenikov > <dmitry.ereminsolenikov@linaro.org> wrote: >> On 05.05.2017 11:17, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> >>> >>>> -----Original Message----- >>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of >>>> Github ODP bot >>>> Sent: Thursday, May 04, 2017 8:00 PM >>>> To: lng-odp@lists.linaro.org >>>> Subject: [lng-odp] [PATCH API-NEXT v1 2/2] api: ipsec: move soft limits >>>> expiration to flags, rather than errors >>>> >>>> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> >>>> >>>> Soft limit expiration isn't an error per se. It does not mean, that we >>>> received invalid or unprocessed packet. They look more like flags, >>>> noting that soft limit on this SA was expired. >>> >>> Advantage of keeping those among error flags, is the easy/fast check in the common case: >>> >>> Only check (error.u64 == 0) to see that everything is OK vs. >> >> Well, that is exactly my point for moving soft_exp out of error >> bitfield. Soft expiration is not an error. The packet was processed >> successfully. In case of outbound inline processing, it was even sent to >> pktio. >> >> I have following code scenarios for the application (leaving alone time >> based expiration, which should be rethought, especially wrt. SYNC >> processing): >> >> - SYNC or ASYNC: application checks status.flag.soft_exp_* to check if >> it needs to rekey. Additional care should be applied so that rekeying is >> not started multiple times for the same SA, because further packet >> status will also contains soft_exp_* flag set on. >> >> - ASYNC or INLINE: application receives IPSEC_STATUS event for this SA >> queue and then starts rekeying based on that event. >> >> I should probably note that there is no direct need to introduce >> IPSEC_STATUS events for hard timeouts, since they will end up as errors. > > Hard and soft timeouts have the same problem in that they are > independent of any packet. So if no packets are being processed you > might never notice that a hard timeout has passed. Agree we need to > discuss this next week, but treating all such expirations uniformly as > async events seems consistent and straightforward. This is for bytes and packets expiry, as stated above. -- With best wishes Dmitry
diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h index e83494d..384c43d 100644 --- a/include/odp/api/spec/ipsec.h +++ b/include/odp/api/spec/ipsec.h @@ -860,15 +860,6 @@ typedef struct odp_ipsec_op_status_t { /** Packet does not fit into the given MTU size */ uint32_t mtu : 1; - /** Soft lifetime expired: seconds */ - uint32_t soft_exp_sec : 1; - - /** Soft lifetime expired: bytes */ - uint32_t soft_exp_bytes : 1; - - /** Soft lifetime expired: packets */ - uint32_t soft_exp_packets : 1; - /** Hard lifetime expired: seconds */ uint32_t hard_exp_sec : 1; @@ -894,6 +885,15 @@ typedef struct odp_ipsec_op_status_t { union { /** Status flags */ struct { + /** Soft lifetime expired: seconds */ + uint32_t soft_exp_sec : 1; + + /** Soft lifetime expired: bytes */ + uint32_t soft_exp_bytes : 1; + + /** Soft lifetime expired: packets */ + uint32_t soft_exp_packets : 1; + /** Packet was processed in inline mode */ uint32_t inline_mode : 1;