[ovs-dev] [PATCH v4] netlink-socket: Exit NL transaction loop when EINVAL is returned

[Eitan Eliahu]

As I understand the semantics, we need to return an error and bail out only when there a "system level" error which equivalent to a transport error or disconnection in a standard IPC. All transaction based errors should return a success by the I/O control and an NL error message should be  built in the reply buffer.
Incrementing the *done variable would be closer to what we want to achieve but until we aligned the driver with this policy this change would not be complete. For now, we just want to prevent the state where we enter an infinite loop.
Thanks,
Eitan

-----Original Message-----
From: Sorin Vinturis [mailto:svinturis at cloudbasesolutions.com] 
Sent: Wednesday, April 15, 2015 9:10 PM
To: Nithin Raju; Eitan Eliahu
Cc: dev at openvswitch.org
Subject: RE: [ovs-dev] [PATCH v4] netlink-socket: Exit NL transaction loop when EINVAL is returned

Returning error on driver request failure would exit the while loop no matter if the done counter is incremented or not.
So either we return error and exit immediately the while loop, or increment the done counter and continue to discuss to an unresponsive driver until the transactions are consumed.

-----Original Message-----
From: Nithin Raju [mailto:nithin at vmware.com] 
Sent: Thursday, 16 April, 2015 00:22
To: Eitan Eliahu
Cc: Sorin Vinturis; dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4] netlink-socket: Exit NL transaction loop when EINVAL is returned

> On Apr 15, 2015, at 1:49 PM, Eitan Eliahu <eliahue at vmware.com> wrote:
> 
> If we remove the increment of the transaction than the whole sequence of transaction will be lost.

This is not true. We increment ‘*done’ after each successful transaction:
48         /* Count the number of successful transactions. */ 
49         (*done)++;                                         

> This should be done only after we change the driver to never fail in case the transaction fail. The current driver fails also when there are transaction level erros and when there are some other "temporary" errors. I would prefer to move to the next transaction in that case rather than dump the whole sequence. 

Yes, the driver needs to be updated to return STATUS_SUCCESS in most cases, unless there are issues with the genetlink header itself. If there’s such an error value returned by the kernel, instead of checking 'error = EINVAL’, we should check for GetLastError(). Otherwise, nl_sock_transact_multiple__() should behave as though no error occurred.

-- Nithin