| Message ID | 1399614455-10551-1-git-send-email-xufeng.zhang@windriver.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Xufeng Zhang <xufeng.zhang@windriver.com> Date: Fri, 9 May 2014 13:47:35 +0800 > Current implementation for pfkey_sendmsg() always return success > no matter whether or not error happens during this syscall, > this is incompatible with the general send()/sendmsg() API: > man send > RETURN VALUE > On success, these calls return the number of characters sent. > On error, -1 is returned, and errno is set appropriately. > > One side effect this problem introduces is that we can't determine > when to resend the message when the previous send() fails because > it was interrupted by signals. > We detect such a problem when racoon is sending SADBADD message to > add SAD entry in the kernel, but sometimes kernel is responding with > "Interrupted system call"(-EINTR) error. > > Check the send implementation of strongswan, it has below logic: > pfkey_send_socket() > { > ... > while (TRUE) > { > len = send(socket, in, in_len, 0); > > if (len != in_len) > { > case EINTR: > /* interrupted, try again */ > continue; > ... > } > } > ... > } > So it makes sense to return errors for send() syscall. > > Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com> I disagree. If pfkey_error() is successful, the error will be reported in the AF_KEY message that is broadcast, there is no reason for sendmsg to return an error. The message was sucessfully sent, there was no problem with it's passage into the AF_KEY layer. Like netlink, operational responses come in packets, not error codes. However, if pfkey_error() fails, we must do pass back the original error code because it's a last ditch effort to prevent information from being lost. That's why 'err' must be preserved when pfkey_error() returns zero. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/12/2014 01:11 PM, David Miller wrote: > >> So it makes sense to return errors for send() syscall. >> >> Signed-off-by: Xufeng Zhang<xufeng.zhang@windriver.com> >> > I disagree. > > If pfkey_error() is successful, the error will be reported in the AF_KEY > message that is broadcast, there is no reason for sendmsg to return an > error. The message was sucessfully sent, there was no problem with it's > passage into the AF_KEY layer. > > Like netlink, operational responses come in packets, not error codes. > > However, if pfkey_error() fails, we must do pass back the original > error code because it's a last ditch effort to prevent information > from being lost. > > That's why 'err' must be preserved when pfkey_error() returns zero. > I know what you mean, but isn't the kernel API aimed to facilitate the implementation of user space? Since send the message to the kernel and receive the error report message are asynchronous, I don't think it's easy to recover from the EINTR error by parsing the error report message. Thanks, Xufeng -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Xufeng Zhang <xufeng.zhang@windriver.com> Date: Mon, 12 May 2014 14:05:09 +0800 > I know what you mean, but isn't the kernel API aimed to facilitate the > implementation of user space? Yes, but as I stated the communication is message oriented and this includes the errors. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/key/af_key.c b/net/key/af_key.c index f3c8307..9e4bc8c 100644 --- a/net/key/af_key.c +++ b/net/key/af_key.c @@ -335,7 +335,7 @@ static int pfkey_error(const struct sadb_msg *orig, int err, struct sock *sk) pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ONE, sk, sock_net(sk)); - return 0; + return -err; } static const u8 sadb_ext_min_len[] = { @@ -3644,8 +3644,8 @@ static int pfkey_sendmsg(struct kiocb *kiocb, mutex_unlock(&net->xfrm.xfrm_cfg_mutex); out: - if (err && hdr && pfkey_error(hdr, err, sk) == 0) - err = 0; + if (err && hdr) + err = pfkey_error(hdr, err, sk); kfree_skb(skb); return err ? : len;
Current implementation for pfkey_sendmsg() always return success no matter whether or not error happens during this syscall, this is incompatible with the general send()/sendmsg() API: man send RETURN VALUE On success, these calls return the number of characters sent. On error, -1 is returned, and errno is set appropriately. One side effect this problem introduces is that we can't determine when to resend the message when the previous send() fails because it was interrupted by signals. We detect such a problem when racoon is sending SADBADD message to add SAD entry in the kernel, but sometimes kernel is responding with "Interrupted system call"(-EINTR) error. Check the send implementation of strongswan, it has below logic: pfkey_send_socket() { ... while (TRUE) { len = send(socket, in, in_len, 0); if (len != in_len) { case EINTR: /* interrupted, try again */ continue; ... } } ... } So it makes sense to return errors for send() syscall. Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com> --- net/key/af_key.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)