Hi Bjorn,
On 9/1/2023 7:41 PM, Bjorn Andersson wrote:
On Fri, Sep 01, 2023 at 03:50:20PM +0530, Sricharan Ramabadhran wrote:
From: Vignesh Viswanathan <quic_viswanat@xxxxxxxxxxx>
If qrtr and some other process try to bind to the QMI Control port at
It's unclear to me which "qrtr" is being referred here, could it be
"qrtr-ns", if so could we express that as "the name server".
yes, its name-space server. Will put it explicitly.
We only allow one bind on the qrtr control port, so could it be that
"QMI Control port" refer to the control socket in the userspace QC[CS]I
libraries, if so that's just any random socket sending out a control
message.
Can we please rephrase this problem description to make the chain of
events clear?
In this case we are talking about a client connecting/sending to QRTR
socket and the 'NS' doing a qrtr_bind during its init. There is
possibility that a client tries to send to the 'NS' before processing
the ENETRESET. In the case of a NEW_SERVER control message will
reach the 'NS' and be forwarded to the firmware. The client will then
process the ENETRESET closing and re-opening the socket which triggers
a DEL_SERVER and then a second NEW_SERVER. This scenario will give an
unnecessary disconnect to the clients on the firmware who were able to
initialize on the first NEW_SERVER.
Also about the patch #2, i guess QRTR_BYE/DEL_PROC should also be
broadcasted, right now we are only listening on DEL_PROC sent by
legacy kernels like SDX modems. Without that modem SSR feature is
broken on IPQ + SDX targets.
the same time, NEW_SERVER might come before ENETRESET is given to the
socket. This might cause a socket down/up when ENETRESET is received as
per the protocol and this triggers a DEL_SERVER and a second NEW_SERVER.
In order to prevent such messages from stale sockets being sent, check
if ENETRESET has been set on the socket and drop the packet.
Signed-off-by: Chris Lew <quic_clew@xxxxxxxxxxx>
Signed-off-by: Vignesh Viswanathan <quic_viswanat@xxxxxxxxxxx>
The first person to certify the patch's origin, must be the author, and
when you pick the patch to send it you need to add your s-o-b.
So please fix the author, and add your s-o-b.
ok sure, will fix.
Let's add Chris to the recipients list as well.
ok.
---
net/qrtr/af_qrtr.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 41ece61..26197a0 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -851,6 +851,7 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
{
struct qrtr_sock *ipc;
struct qrtr_cb *cb;
+ struct sock *sk = skb->sk;
ipc = qrtr_port_lookup(to->sq_port);
if (!ipc || &ipc->sk == skb->sk) { /* do not send to self */
@@ -860,6 +861,15 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
return -ENODEV;
}
+ /* Keep resetting NETRESET until socket is closed */
+ if (sk && sk->sk_err == ENETRESET) {
+ sk->sk_err = ENETRESET;
Isn't this line unnecessary?
yup, will be removed in V2.
Regards,
Sricharan
