Message-ID: <1ae01ab918876941dc57d01d4c2f1d7376dda87b.camel@egluetechnologies.com>
Subject: Re: [PATCH v2] can: j1939: do not wait 250 ms if the same addr was
 already claimed
From:   Devid Antonio Filoni <devid.filoni@egluetechnologies.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>,
        Robin van der Gracht <robin@protonic.nl>,
        Oliver Hartkopp <socketcan@hartkopp.net>,
        Marc Kleine-Budde <mkl@pengutronix.de>
Cc:     Oleksij Rempel <linux@rempel-privat.de>, kernel@pengutronix.de,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-can@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Tue, 07 Feb 2023 14:50:15 +0100
In-Reply-To: <20221126102840.GA21761@pengutronix.de>
References: <20221124051611.GA7870@pengutronix.de>
         <20221125170418.34575-1-devid.filoni@egluetechnologies.com>
         <20221126102840.GA21761@pengutronix.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

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", stat=
es:
> >   d) No CF shall begin, or resume, transmission on the network until 25=
0
> >      ms after it has successfully claimed an address except when
> >      responding to a request for address-claimed.
> >=20
> > 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.
> >=20
> > 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.
> >=20
> > 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
> >=20
> > Taking the above arguments into account, the 250 ms wait is requested
> > only during network initialization.
> >=20
> > 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).
> >=20
> > Signed-off-by: Devid Antonio Filoni <devid.filoni@egluetechnologies.com=
>
>=20
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
>=20
> > ---
> >  v1 -> v2: Added ISO 11783-5 standard references
> >=20
> >  net/can/j1939/address-claim.c | 40 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >=20
> > diff --git a/net/can/j1939/address-claim.c b/net/can/j1939/address-clai=
m.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 *pr=
iv, struct sk_buff *skb)
> >  	 * leaving this function.
> >  	 */
> >  	ecu =3D j1939_ecu_get_by_name_locked(priv, name);
> > +
> > +	if (ecu && ecu->addr =3D=3D 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 =3D j1939_ecu_create_locked(priv, name);
> > =20
> > --=20
> > 2.34.1
> >=20
> >=20
>=20

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

Thank you,
Devid