[PATCH net] amt: fix gateway mode stuck

[PATCH net] amt: fix gateway mode stuck

[Taehee Yoo]

If a gateway can not receive any response to requests from a relay,
gateway resets status from SENT_REQUEST to INIT and variable about a
relay as well. And then it should start the full establish step
from sending a discovery message and receiving advertisement message.
But, after failure in amt_req_work() it continues sending a request
message step with flushed(invalid) relay information and sets SENT_REQUEST.
So, a gateway can't be established with a relay.
In order to avoid this situation, it stops sending the request message
step if it fails.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/amt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 10455c9b9da0..6ce2ecd07640 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -943,7 +943,7 @@ static void amt_req_work(struct work_struct *work)
 	if (amt->status < AMT_STATUS_RECEIVED_ADVERTISEMENT)
 		goto out;
 
-	if (amt->req_cnt++ > AMT_MAX_REQ_COUNT) {
+	if (amt->req_cnt > AMT_MAX_REQ_COUNT) {
 		netdev_dbg(amt->dev, "Gateway is not ready");
 		amt->qi = AMT_INIT_REQ_TIMEOUT;
 		amt->ready4 = false;
@@ -951,13 +951,15 @@ static void amt_req_work(struct work_struct *work)
 		amt->remote_ip = 0;
 		__amt_update_gw_status(amt, AMT_STATUS_INIT, false);
 		amt->req_cnt = 0;
+		goto out;
 	}
 	spin_unlock_bh(&amt->lock);
 
 	amt_send_request(amt, false);
 	amt_send_request(amt, true);
-	amt_update_gw_status(amt, AMT_STATUS_SENT_REQUEST, true);
 	spin_lock_bh(&amt->lock);
+	__amt_update_gw_status(amt, AMT_STATUS_SENT_REQUEST, true);
+	amt->req_cnt++;
 out:
 	exp = min_t(u32, (1 * (1 << amt->req_cnt)), AMT_MAX_REQ_TIMEOUT);
 	mod_delayed_work(amt_wq, &amt->req_wq, msecs_to_jiffies(exp * 1000));
@@ -2696,9 +2698,10 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 				err = true;
 				goto drop;
 			}
-			if (amt_advertisement_handler(amt, skb))
+			err = amt_advertisement_handler(amt, skb);
+			if (err)
 				amt->dev->stats.rx_dropped++;
-			goto out;
+			break;
 		case AMT_MSG_MULTICAST_DATA:
 			if (iph->saddr != amt->remote_ip) {
 				netdev_dbg(amt->dev, "Invalid Relay IP\n");
-- 
2.17.1

Re: [PATCH net] amt: fix gateway mode stuck

[Jakub Kicinski]

On Sat, 14 May 2022 13:13:46 +0000 Taehee Yoo wrote:
> -			if (amt_advertisement_handler(amt, skb))
> +			err = amt_advertisement_handler(amt, skb);
> +			if (err)
>  				amt->dev->stats.rx_dropped++;
> -			goto out;
> +			break;

There's another amt->dev->stats.rx_dropped++; before the end of this
function which now won't be skipped, I think you're counting twice.

Re: [PATCH net] amt: fix gateway mode stuck

[Taehee Yoo]

On 5/17/22 08:10, Jakub Kicinski wrote:

Hi Jakub,

Thanks a lot for your review!

> On Sat, 14 May 2022 13:13:46 +0000 Taehee Yoo wrote:
>> -			if (amt_advertisement_handler(amt, skb))
>> +			err = amt_advertisement_handler(amt, skb);
>> +			if (err)
>>   				amt->dev->stats.rx_dropped++;
>> -			goto out;
>> +			break;
> 
> There's another amt->dev->stats.rx_dropped++; before the end of this
> function which now won't be skipped, I think you're counting twice.
This is intended.
It skips a remaining handling of advertisement message.
So, I think a memory leak would occur at this point, so I added.

Thanks!
Taehee Yoo