| Message ID | 20210706115758.11196-1-o.rempel@pengutronix.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/2] net: j1939: rename J1939_ERRQUEUE_* to J1939_ERRQUEUE_TX_* | expand |
On Tue, 6 Jul 2021 13:57:58 +0200 Oleksij Rempel <o.rempel@pengutronix.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@pengutronix.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,
On Tue, Jul 06, 2021 at 03:28:34PM +0200, David Jander wrote: > On Tue, 6 Jul 2021 13:57:58 +0200 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > -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...? done > > + nla_total_size(sizeof(u64)) + /* J1939_NLA_DEST_NAME */ > > + nla_total_size(sizeof(u64)) + /* J1939_NLA_SRC_NAME */ > > + 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? ack, done Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h index 12369b604ce9..93b8ad7f7d04 100644 --- a/net/can/j1939/j1939-priv.h +++ b/net/can/j1939/j1939-priv.h @@ -20,9 +20,9 @@ struct j1939_session; enum j1939_sk_errqueue_type { - J1939_ERRQUEUE_ACK, - J1939_ERRQUEUE_SCHED, - J1939_ERRQUEUE_ABORT, + J1939_ERRQUEUE_TX_ACK, + J1939_ERRQUEUE_TX_SCHED, + J1939_ERRQUEUE_TX_ABORT, }; /* j1939 devices */ diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c index 56aa66147d5a..c2bf1c02597e 100644 --- a/net/can/j1939/socket.c +++ b/net/can/j1939/socket.c @@ -961,7 +961,7 @@ void j1939_sk_errqueue(struct j1939_session *session, serr = SKB_EXT_ERR(skb); memset(serr, 0, sizeof(*serr)); switch (type) { - case J1939_ERRQUEUE_ACK: + case J1939_ERRQUEUE_TX_ACK: if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK)) { kfree_skb(skb); return; @@ -972,7 +972,7 @@ void j1939_sk_errqueue(struct j1939_session *session, serr->ee.ee_info = SCM_TSTAMP_ACK; state = "ACK"; break; - case J1939_ERRQUEUE_SCHED: + case J1939_ERRQUEUE_TX_SCHED: if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED)) { kfree_skb(skb); return; @@ -983,7 +983,7 @@ void j1939_sk_errqueue(struct j1939_session *session, serr->ee.ee_info = SCM_TSTAMP_SCHED; state = "SCH"; break; - case J1939_ERRQUEUE_ABORT: + 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; diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index e09d087ba240..362cf38cacca 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -261,9 +261,9 @@ 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_ABORT); + j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ABORT); else - j1939_sk_errqueue(session, J1939_ERRQUEUE_ACK); + j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_ACK); netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session); @@ -1026,7 +1026,7 @@ static int j1939_simple_txnext(struct j1939_session *session) if (ret) return ret; - j1939_sk_errqueue(session, J1939_ERRQUEUE_SCHED); + j1939_sk_errqueue(session, J1939_ERRQUEUE_TX_SCHED); j1939_sk_queue_activate_next(session); return 0; @@ -1405,7 +1405,7 @@ j1939_xtp_rx_cts_one(struct j1939_session *session, struct sk_buff *skb) if (session->transmission) { if (session->pkt.tx_acked) j1939_sk_errqueue(session, - J1939_ERRQUEUE_SCHED); + J1939_ERRQUEUE_TX_SCHED); j1939_session_txtimer_cancel(session); j1939_tp_schedule_txtimer(session, 0); }
Prepare the world for the J1939_ERRQUEUE_RX_ version Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- net/can/j1939/j1939-priv.h | 6 +++--- net/can/j1939/socket.c | 6 +++--- net/can/j1939/transport.c | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-)