| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 6 Jul 2021 15:28:34 +0200
From: David Jander <david@...tonic.nl>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: dev.kurt@...dijck-laurijssen.be, mkl@...gutronix.de,
wg@...ndegger.com, kernel@...gutronix.de,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
Zhang Changzhong <zhangchangzhong@...wei.com>
Subject: Re: [PATCH v1 2/2] net: j1939: extend UAPI to notify about RX
status
On Tue, 6 Jul 2021 13:57:58 +0200
Oleksij Rempel <o.rempel@...gutronix.de> wrote:
> To be able to create applications with user friendly feedback, we need be
> able to provide receive status information.
>
> Typical ETP transfer may take seconds or even hours. To give user some
> clue or show a progress bar, the stack should push status updates.
> Same as for the TX information, the socket error queue will be used with
> following new signals:
> - J1939_EE_INFO_RX_RTS - received and accepted request to send signal.
> - J1939_EE_INFO_RX_DPO - received data package offset signal
> - J1939_EE_INFO_RX_ABORT - RX session was aborted
>
> Instead of completion signal, user will get data package.
> To activate this signals, application should set
> SOF_TIMESTAMPING_RX_SOFTWARE to the SO_TIMESTAMPING socket option. This
> will avoid unpredictable application behavior for the old software.
>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> include/uapi/linux/can/j1939.h | 9 +++
> net/can/j1939/j1939-priv.h | 4 +
> net/can/j1939/socket.c | 135 +++++++++++++++++++++++++--------
> net/can/j1939/transport.c | 22 +++++-
> 4 files changed, 136 insertions(+), 34 deletions(-)
>
> diff --git a/include/uapi/linux/can/j1939.h b/include/uapi/linux/can/j1939.h
> index df6e821075c1..dc2a8e15c0b7 100644
> --- a/include/uapi/linux/can/j1939.h
> +++ b/include/uapi/linux/can/j1939.h
> @@ -78,11 +78,20 @@ enum {
> enum {
> J1939_NLA_PAD,
> J1939_NLA_BYTES_ACKED,
> + J1939_NLA_TOTAL_SIZE,
> + J1939_NLA_DEST_ADDR,
> + J1939_NLA_DEST_NAME,
> + J1939_NLA_SRC_ADDR,
> + J1939_NLA_SRC_NAME,
> + J1939_NLA_PGN,
> };
>
> enum {
> J1939_EE_INFO_NONE,
> J1939_EE_INFO_TX_ABORT,
> + J1939_EE_INFO_RX_RTS,
> + J1939_EE_INFO_RX_DPO,
> + J1939_EE_INFO_RX_ABORT,
> };
>
> struct j1939_filter {
> diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
> index 93b8ad7f7d04..f6df20808f5e 100644
> --- a/net/can/j1939/j1939-priv.h
> +++ b/net/can/j1939/j1939-priv.h
> @@ -23,6 +23,9 @@ enum j1939_sk_errqueue_type {
> J1939_ERRQUEUE_TX_ACK,
> J1939_ERRQUEUE_TX_SCHED,
> J1939_ERRQUEUE_TX_ABORT,
> + J1939_ERRQUEUE_RX_RTS,
> + J1939_ERRQUEUE_RX_DPO,
> + J1939_ERRQUEUE_RX_ABORT,
> };
>
> /* j1939 devices */
> @@ -87,6 +90,7 @@ struct j1939_priv {
> struct list_head j1939_socks;
>
> struct kref rx_kref;
> + u32 rx_tskey;
> };
>
> void j1939_ecu_put(struct j1939_ecu *ecu);
> diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
> index c2bf1c02597e..30b99897c473 100644
> --- a/net/can/j1939/socket.c
> +++ b/net/can/j1939/socket.c
> @@ -902,20 +902,33 @@ static struct sk_buff *j1939_sk_alloc_skb(struct net_device *ndev,
> return NULL;
> }
>
> -static size_t j1939_sk_opt_stats_get_size(void)
> +static size_t j1939_sk_opt_stats_get_size(enum j1939_sk_errqueue_type type)
> {
> - return
> - nla_total_size(sizeof(u32)) + /* J1939_NLA_BYTES_ACKED */
> - 0;
> + switch (type) {
> + case J1939_ERRQUEUE_RX_RTS:
> + return
> + nla_total_size(sizeof(u32)) + /* J1939_NLA_BYTES_ALL */
> + nla_total_size(sizeof(u64)) + /* J1939_NLA_DEST_ADDR */
> + nla_total_size(sizeof(u64)) + /* J1939_NLA_SRC_ADDR */
DST and SRC address are u8...?
> + nla_total_size(sizeof(u64)) + /* J1939_NLA_DEST_NAME */
> + nla_total_size(sizeof(u64)) + /* J1939_NLA_SRC_NAME */
> + nla_total_size(sizeof(u32)) + /* J1939_NLA_PGN */
> + 0;
> + default:
> + return
> + nla_total_size(sizeof(u32)) + /* J1939_NLA_BYTES_ACKED */
> + 0;
> + }
> }
>
> static struct sk_buff *
> -j1939_sk_get_timestamping_opt_stats(struct j1939_session *session)
> +j1939_sk_get_timestamping_opt_stats(struct j1939_session *session,
> + enum j1939_sk_errqueue_type type)
> {
> struct sk_buff *stats;
> u32 size;
>
> - stats = alloc_skb(j1939_sk_opt_stats_get_size(), GFP_ATOMIC);
> + stats = alloc_skb(j1939_sk_opt_stats_get_size(type), GFP_ATOMIC);
> if (!stats)
> return NULL;
>
> @@ -925,32 +938,67 @@ j1939_sk_get_timestamping_opt_stats(struct j1939_session *session)
> size = min(session->pkt.tx_acked * 7,
> session->total_message_size);
>
> - nla_put_u32(stats, J1939_NLA_BYTES_ACKED, size);
> + switch (type) {
> + case J1939_ERRQUEUE_RX_RTS:
> + nla_put_u32(stats, J1939_NLA_TOTAL_SIZE,
> + session->total_message_size);
> + nla_put_u32(stats, J1939_NLA_PGN,
> + session->skcb.addr.pgn);
> + nla_put_u64_64bit(stats, J1939_NLA_SRC_NAME,
> + session->skcb.addr.src_name, J1939_NLA_PAD);
> + nla_put_u64_64bit(stats, J1939_NLA_DEST_NAME,
> + session->skcb.addr.dst_name, J1939_NLA_PAD);
> + nla_put_u8(stats, J1939_NLA_SRC_ADDR,
> + session->skcb.addr.sa);
> + nla_put_u8(stats, J1939_NLA_DEST_ADDR,
> + session->skcb.addr.da);
See above.
Also, shouldn't the order of these be the same as in
j1939_sk_opt_stats_get_size()... for readability?
> + break;
> + default:
> + nla_put_u32(stats, J1939_NLA_BYTES_ACKED, size);
> + }
>
> return stats;
> }
>
> -void j1939_sk_errqueue(struct j1939_session *session,
> - enum j1939_sk_errqueue_type type)
> +static void __j1939_sk_errqueue(struct j1939_session *session, struct sock *sk,
> + enum j1939_sk_errqueue_type type)
> {
> struct j1939_priv *priv = session->priv;
> - struct sock *sk = session->sk;
> struct j1939_sock *jsk;
> struct sock_exterr_skb *serr;
> struct sk_buff *skb;
> char *state = "UNK";
> int err;
>
> - /* currently we have no sk for the RX session */
> - if (!sk)
> - return;
> -
> jsk = j1939_sk(sk);
>
> if (!(jsk->state & J1939_SOCK_ERRQUEUE))
> return;
>
> - skb = j1939_sk_get_timestamping_opt_stats(session);
> + switch (type) {
> + case J1939_ERRQUEUE_TX_ACK:
> + if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK))
> + return;
> + break;
> + case J1939_ERRQUEUE_TX_SCHED:
> + if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED))
> + return;
> + break;
> + case J1939_ERRQUEUE_TX_ABORT:
> + break;
> + case J1939_ERRQUEUE_RX_RTS:
> + fallthrough;
> + case J1939_ERRQUEUE_RX_DPO:
> + fallthrough;
> + case J1939_ERRQUEUE_RX_ABORT:
> + if (!(sk->sk_tsflags & SOF_TIMESTAMPING_RX_SOFTWARE))
> + return;
> + break;
> + default:
> + netdev_err(priv->ndev, "Unknown errqueue type %i\n", type);
> + }
> +
> + skb = j1939_sk_get_timestamping_opt_stats(session, type);
> if (!skb)
> return;
>
> @@ -962,35 +1010,41 @@ void j1939_sk_errqueue(struct j1939_session *session,
> memset(serr, 0, sizeof(*serr));
> switch (type) {
> case J1939_ERRQUEUE_TX_ACK:
> - if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK)) {
> - kfree_skb(skb);
> - return;
> - }
> -
> serr->ee.ee_errno = ENOMSG;
> serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
> serr->ee.ee_info = SCM_TSTAMP_ACK;
> - state = "ACK";
> + state = "TX ACK";
> break;
> case J1939_ERRQUEUE_TX_SCHED:
> - if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED)) {
> - kfree_skb(skb);
> - return;
> - }
> -
> serr->ee.ee_errno = ENOMSG;
> serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
> serr->ee.ee_info = SCM_TSTAMP_SCHED;
> - state = "SCH";
> + state = "TX SCH";
> break;
> case J1939_ERRQUEUE_TX_ABORT:
> serr->ee.ee_errno = session->err;
> serr->ee.ee_origin = SO_EE_ORIGIN_LOCAL;
> serr->ee.ee_info = J1939_EE_INFO_TX_ABORT;
> - state = "ABT";
> + state = "TX ABT";
> + break;
> + case J1939_ERRQUEUE_RX_RTS:
> + serr->ee.ee_errno = ENOMSG;
> + serr->ee.ee_origin = SO_EE_ORIGIN_LOCAL;
> + serr->ee.ee_info = J1939_EE_INFO_RX_RTS;
> + state = "RX RTS";
> + break;
> + case J1939_ERRQUEUE_RX_DPO:
> + serr->ee.ee_errno = ENOMSG;
> + serr->ee.ee_origin = SO_EE_ORIGIN_LOCAL;
> + serr->ee.ee_info = J1939_EE_INFO_RX_DPO;
> + state = "RX DPO";
> + break;
> + case J1939_ERRQUEUE_RX_ABORT:
> + serr->ee.ee_errno = session->err;
> + serr->ee.ee_origin = SO_EE_ORIGIN_LOCAL;
> + serr->ee.ee_info = J1939_EE_INFO_RX_ABORT;
> + state = "RX ABT";
> break;
> - default:
> - netdev_err(priv->ndev, "Unknown errqueue type %i\n", type);
> }
>
> serr->opt_stats = true;
> @@ -1005,6 +1059,27 @@ void j1939_sk_errqueue(struct j1939_session *session,
> kfree_skb(skb);
> };
>
> +void j1939_sk_errqueue(struct j1939_session *session,
> + enum j1939_sk_errqueue_type type)
> +{
> + struct j1939_priv *priv = session->priv;
> + struct j1939_sock *jsk;
> +
> + if (session->sk) {
> + /* send TX notifications to the socket of origin */
> + __j1939_sk_errqueue(session, session->sk, type);
> + return;
> + }
> +
> + /* spread RX notifications to all sockets subscribed to this session */
> + spin_lock_bh(&priv->j1939_socks_lock);
> + list_for_each_entry(jsk, &priv->j1939_socks, list) {
> + if (j1939_sk_recv_match_one(jsk, &session->skcb))
> + __j1939_sk_errqueue(session, &jsk->sk, type);
> + }
> + spin_unlock_bh(&priv->j1939_socks_lock);
> +};
> +
> void j1939_sk_send_loop_abort(struct sock *sk, int err)
> {
> sk->sk_err = err;
> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
> index 362cf38cacca..cb358646e382 100644
> --- a/net/can/j1939/transport.c
> +++ b/net/can/j1939/transport.c
> @@ -260,10 +260,14 @@ static void __j1939_session_drop(struct j1939_session *session)
>
> static void j1939_session_destroy(struct j1939_session *session)
> {
> - if (session->err)
> - j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ABORT);
> - else
> - j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ACK);
> + if (session->transmission) {
> + if (session->err)
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ABORT);
> + else
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ACK);
> + } else if (session->err) {
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_RX_ABORT);
> + }
>
> netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session);
>
> @@ -1087,6 +1091,8 @@ static void __j1939_session_cancel(struct j1939_session *session,
>
> if (session->sk)
> j1939_sk_send_loop_abort(session->sk, session->err);
> + else
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_RX_ABORT);
> }
>
> static void j1939_session_cancel(struct j1939_session *session,
> @@ -1297,6 +1303,8 @@ static void j1939_xtp_rx_abort_one(struct j1939_priv *priv, struct sk_buff *skb,
> session->err = j1939_xtp_abort_to_errno(priv, abort);
> if (session->sk)
> j1939_sk_send_loop_abort(session->sk, session->err);
> + else
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_RX_ABORT);
> j1939_session_deactivate_activate_next(session);
>
> abort_put:
> @@ -1597,6 +1605,9 @@ j1939_session *j1939_xtp_rx_rts_session_new(struct j1939_priv *priv,
> session->pkt.rx = 0;
> session->pkt.tx = 0;
>
> + session->tskey = priv->rx_tskey++;
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_RX_RTS);
> +
> WARN_ON_ONCE(j1939_session_activate(session));
>
> return session;
> @@ -1719,6 +1730,9 @@ static void j1939_xtp_rx_dpo_one(struct j1939_session *session,
> session->pkt.dpo = j1939_etp_ctl_to_packet(skb->data);
> session->last_cmd = dat[0];
> j1939_tp_set_rxtimeout(session, 750);
> +
> + if (!session->transmission)
> + j1939_sk_errqueue(session, J1939_ERRQUEUE_RX_DPO);
> }
>
> static void j1939_xtp_rx_dpo(struct j1939_priv *priv, struct sk_buff *skb,
Best regards,
--
David Jander
Protonic Holland.
Powered by blists - more mailing lists