netdev - Re: [PATCH net-next] net: qualcomm: rmnet: Add side band flow control support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49fed647-f8aa-857c-4edc-d38cf6a793d7@linux.dev>
Date: Thu, 21 Sep 2023 11:51:00 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Subash Abhinov Kasiviswanathan <quic_subashab@...cinc.com>,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, netdev@...r.kernel.org
Cc: Sean Tranchetti <quic_stranche@...cinc.com>
Subject: Re: [PATCH net-next] net: qualcomm: rmnet: Add side band flow control
 support

On 20/09/2023 01:33, Subash Abhinov Kasiviswanathan wrote:
> Individual rmnet devices map to specific network types such as internet,
> multimedia messaging services, IP multimedia subsystem etc. Each of
> these network types may support varying quality of service for different
> bearers or traffic types.
> 
> The physical device interconnect to radio hardware may support a
> higher data rate than what is actually supported by the radio network.
> Any packets transmitted to the radio hardware which exceed the radio
> network data rate limit maybe dropped. This patch tries to minimize the
> loss of packets by adding support for bearer level flow control within a
> rmnet device by ensuring that the packets transmitted do not exceed the
> limit allowed by the radio network.
> 
> In order to support multiple bearers, rmnet must be created as a
> multiqueue TX netdevice. Radio hardware communicates the supported
> bearer information for a given network via side band signalling.
> Consider the following mapping -
> 
> IPv4 UDP port 1234 - Mark 0x1001 - Queue 1
> IPv6 TCP port 2345 - Mark 0x2001 - Queue 2
> 
> iptables can be used to install filters which mark packets matching these
> specific traffic patterns and the RMNET_QUEUE_MAPPING_ADD operation can
> then be to install the mapping of the mark to the specific txqueue.
> 
> If the traffic limit is exceeded for a particular bearer, radio hardware
> would notify that the bearer cannot accept more packets and the
> corresponding txqueue traffic can be stopped using RMNET_QUEUE_DISABLE.
> 
> Conversely, if radio hardware can send more traffic for a particular
> bearer, RMNET_QUEUE_ENABLE can be used to allow traffic on that
> particular txqueue. RMNET_QUEUE_MAPPING_REMOVE can be used to remove the
> mark to queue mapping in case the radio network doesn't support that
> particular bearer any longer.
> 
> Signed-off-by: Sean Tranchetti <quic_stranche@...cinc.com>
> Signed-off-by: Subash Abhinov Kasiviswanathan <quic_subashab@...cinc.com>

[ ... ]

> +	case RMNET_QUEUE_ENABLE:
> +	case RMNET_QUEUE_DISABLE:
> +		p = xa_load(&priv->queue_map, mark);
> +		if (p && xa_is_value(p)) {
> +			txq = xa_is_value(p);

                  should it be xa_to_value(p)? otherwise txq is always 1

> +			if (txq >= dev->num_tx_queues) {
> +				NL_SET_ERR_MSG_MOD(extack,
> +						   "queue mapping limit exceeded");
> +				return -EINVAL;
> +			}
> +
> +			q = netdev_get_tx_queue(dev, txq);
> +			if (unlikely(!q)) {
> +				NL_SET_ERR_MSG_MOD(extack,
> +						   "unsupported queue mapping");
> +				return -EINVAL;
> +			}
> +

[ ... ]

>   
>   struct rmnet_port *rmnet_get_port_rcu(struct net_device *real_dev);
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> index 046b5f7d8e7c..6dfee4a4d634 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> @@ -1,5 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>    *
>    * RMNET Data virtual network driver
>    */
> @@ -158,6 +159,23 @@ static void rmnet_get_stats64(struct net_device *dev,
>   	s->tx_dropped = total_stats.tx_drops;
>   }
>   
> +static u16 rmnet_vnd_select_queue(struct net_device *dev,
> +				  struct sk_buff *skb,
> +				  struct net_device *sb_dev)
> +{
> +	struct rmnet_priv *priv = netdev_priv(dev);
> +	void *p = xa_load(&priv->queue_map, skb->mark);
> +	u8 txq;
> +
> +	if (!p && !xa_is_value(p))
> +		return 0;

The check is meaningless. I believe you were thinking about

if (!p || !xa_is_value(p))

But the main question: is it even possible to get something from xarray
that won't pass the check? queue_map is filled in the same code, there 
is now way (I believe) to change it from anywhere else, right?

> +
> +	txq = xa_to_value(p);
> +
> +	netdev_dbg(dev, "mark %u -> txq %u\n", skb->mark, txq);
> +	return (txq < dev->num_tx_queues) ? txq : 0;
> +}