Re: [PATCH v2] can: j1939: do not wait 250 ms if the same addr was already claimed

[Devid Antonio Filoni]

On Sat, 2022-11-26 at 11:28 +0100, Oleksij Rempel wrote:
> On Fri, Nov 25, 2022 at 06:04:18PM +0100, Devid Antonio Filoni wrote:
> > The ISO 11783-5 standard, in "4.5.2 - Address claim requirements", states:
> >   d) No CF shall begin, or resume, transmission on the network until 250
> >      ms after it has successfully claimed an address except when
> >      responding to a request for address-claimed.
> > 
> > But "Figure 6" and "Figure 7" in "4.5.4.2 - Address-claim
> > prioritization" show that the CF begins the transmission after 250 ms
> > from the first AC (address-claimed) message even if it sends another AC
> > message during that time window to resolve the address contention with
> > another CF.
> > 
> > As stated in "4.4.2.3 - Address-claimed message":
> >   In order to successfully claim an address, the CF sending an address
> >   claimed message shall not receive a contending claim from another CF
> >   for at least 250 ms.
> > 
> > As stated in "4.4.3.2 - NAME management (NM) message":
> >   1) A commanding CF can
> >      d) request that a CF with a specified NAME transmit the address-
> >         claimed message with its current NAME.
> >   2) A target CF shall
> >      d) send an address-claimed message in response to a request for a
> >         matching NAME
> > 
> > Taking the above arguments into account, the 250 ms wait is requested
> > only during network initialization.
> > 
> > Do not restart the timer on AC message if both the NAME and the address
> > match and so if the address has already been claimed (timer has expired)
> > or the AC message has been sent to resolve the contention with another
> > CF (timer is still running).
> > 
> > Signed-off-by: Devid Antonio Filoni <devid.filoni@xxxxxxxxxxxxxxxxxxxxx>
> 
> Acked-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> 
> > ---
> >  v1 -> v2: Added ISO 11783-5 standard references
> > 
> >  net/can/j1939/address-claim.c | 40 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/net/can/j1939/address-claim.c b/net/can/j1939/address-claim.c
> > index f33c47327927..ca4ad6cdd5cb 100644
> > --- a/net/can/j1939/address-claim.c
> > +++ b/net/can/j1939/address-claim.c
> > @@ -165,6 +165,46 @@ static void j1939_ac_process(struct j1939_priv *priv, struct sk_buff *skb)
> >  	 * leaving this function.
> >  	 */
> >  	ecu = j1939_ecu_get_by_name_locked(priv, name);
> > +
> > +	if (ecu && ecu->addr == skcb->addr.sa) {
> > +		/* The ISO 11783-5 standard, in "4.5.2 - Address claim
> > +		 * requirements", states:
> > +		 *   d) No CF shall begin, or resume, transmission on the
> > +		 *      network until 250 ms after it has successfully claimed
> > +		 *      an address except when responding to a request for
> > +		 *      address-claimed.
> > +		 *
> > +		 * But "Figure 6" and "Figure 7" in "4.5.4.2 - Address-claim
> > +		 * prioritization" show that the CF begins the transmission
> > +		 * after 250 ms from the first AC (address-claimed) message
> > +		 * even if it sends another AC message during that time window
> > +		 * to resolve the address contention with another CF.
> > +		 *
> > +		 * As stated in "4.4.2.3 - Address-claimed message":
> > +		 *   In order to successfully claim an address, the CF sending
> > +		 *   an address claimed message shall not receive a contending
> > +		 *   claim from another CF for at least 250 ms.
> > +		 *
> > +		 * As stated in "4.4.3.2 - NAME management (NM) message":
> > +		 *   1) A commanding CF can
> > +		 *      d) request that a CF with a specified NAME transmit
> > +		 *         the address-claimed message with its current NAME.
> > +		 *   2) A target CF shall
> > +		 *      d) send an address-claimed message in response to a
> > +		 *         request for a matching NAME
> > +		 *
> > +		 * Taking the above arguments into account, the 250 ms wait is
> > +		 * requested only during network initialization.
> > +		 *
> > +		 * Do not restart the timer on AC message if both the NAME and
> > +		 * the address match and so if the address has already been
> > +		 * claimed (timer has expired) or the AC message has been sent
> > +		 * to resolve the contention with another CF (timer is still
> > +		 * running).
> > +		 */
> > +		goto out_ecu_put;
> > +	}
> > +
> >  	if (!ecu && j1939_address_is_unicast(skcb->addr.sa))
> >  		ecu = j1939_ecu_create_locked(priv, name);
> >  
> > -- 
> > 2.34.1
> > 
> > 
> 

Hello,
I noticed that this patch has not been integrated in upstream yet. Are
there problems with it?

Thank you,
Devid