diff mbox series

[net-next,v4,04/10] netdev-genl: Add netlink framework functions for queue

Message ID 169658369951.3683.3529038539593903265.stgit@anambiarhost.jf.intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1364 this patch: 1364
netdev/cc_maintainers warning 4 maintainers not CCed: ast@kernel.org edumazet@google.com pabeni@redhat.com davem@davemloft.net
netdev/build_clang fail Errors and warnings before: 17 this patch: 17
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 17 this patch: 17
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 239 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Nambiar, Amritha Oct. 6, 2023, 9:14 a.m. UTC 
Implement the netdev netlink framework functions for
exposing queue information.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 net/core/netdev-genl.c |  207 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 204 insertions(+), 3 deletions(-)

Comments

Jakub Kicinski Oct. 11, 2023, 2:25 a.m. UTC | #1 
On Fri, 06 Oct 2023 02:14:59 -0700 Amritha Nambiar wrote:
> +static int
> +netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
> +			 u32 q_idx, u32 q_type, const struct genl_info *info)
> +{
> +	struct netdev_rx_queue *rxq;
> +	struct netdev_queue *txq;
> +	void *hdr;
> +
> +	hdr = genlmsg_iput(rsp, info);
> +	if (!hdr)
> +		return -EMSGSIZE;
> +
> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_ID, q_idx))
> +		goto nla_put_failure;
> +
> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_TYPE, q_type))
> +		goto nla_put_failure;
> +
> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
> +		goto nla_put_failure;

You can combine these ifs in a single one using ||

> +	switch (q_type) {
> +	case NETDEV_QUEUE_TYPE_RX:
> +		rxq = __netif_get_rx_queue(netdev, q_idx);
> +		if (rxq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,

> +static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
> +				    u32 q_type)
> +{
> +	switch (q_type) {
> +	case NETDEV_QUEUE_TYPE_RX:
> +		if (q_id >= netdev->real_num_rx_queues)
> +			return -EINVAL;
> +		return 0;
> +	case NETDEV_QUEUE_TYPE_TX:
> +		if (q_id >= netdev->real_num_tx_queues)
> +			return -EINVAL;
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;

Doesn't the netlink policy prevent this already?

> +	}
> +}

>  int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
>  {
> -	return -EOPNOTSUPP;
> +	u32 q_id, q_type, ifindex;
> +	struct net_device *netdev;
> +	struct sk_buff *rsp;
> +	int err;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_ID))
> +		return -EINVAL;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_TYPE))
> +		return -EINVAL;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
> +		return -EINVAL;

You can combine these checks in a single if using ||

> +	q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_ID]);
> +
> +	q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_TYPE]);
> +
> +	ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);

No need for the empty lines between these.

> +	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!rsp)
> +		return -ENOMEM;
> +
> +	rtnl_lock();
> +
> +	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
> +	if (netdev)
> +		err  = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);

double space after =

> +	else
> +		err = -ENODEV;
> +
> +	rtnl_unlock();
> +
> +	if (err)
> +		goto err_free_msg;
> +
> +	return genlmsg_reply(rsp, info);
> +
> +err_free_msg:
> +	nlmsg_free(rsp);
> +	return err;
> +}
> +
> +static int
> +netdev_nl_queue_dump_one(struct net_device *netdev, struct sk_buff *rsp,
> +			 const struct genl_info *info, unsigned int *start_rx,
> +			 unsigned int *start_tx)
> +{

Hm. Not sure why you don't operate directly on ctx here.
Why pass the indexes by pointer individually?

> +	int err = 0;
> +	int i;
> +
> +	for (i = *start_rx; i < netdev->real_num_rx_queues;) {
> +		err = netdev_nl_queue_fill_one(rsp, netdev, i,
> +					       NETDEV_QUEUE_TYPE_RX, info);
> +		if (err)
> +			goto out_err;

return, no need to goto if all it does is returns

> +		*start_rx = i++;
> +	}
> +	for (i = *start_tx; i < netdev->real_num_tx_queues;) {
> +		err = netdev_nl_queue_fill_one(rsp, netdev, i,
> +					       NETDEV_QUEUE_TYPE_TX, info);
> +		if (err)
> +			goto out_err;
> +		*start_tx = i++;
> +	}
> +out_err:
> +	return err;
>  }
>  
>  int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> -	return -EOPNOTSUPP;
> +	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
> +	const struct genl_info *info = genl_info_dump(cb);
> +	struct net *net = sock_net(skb->sk);
> +	unsigned int rxq_idx = ctx->rxq_idx;
> +	unsigned int txq_idx = ctx->txq_idx;
> +	struct net_device *netdev;
> +	u32 ifindex = 0;
> +	int err = 0;
> +
> +	if (info->attrs[NETDEV_A_QUEUE_IFINDEX])
> +		ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> +
> +	rtnl_lock();
> +	if (ifindex) {
> +		netdev = __dev_get_by_index(net, ifindex);
> +		if (netdev)
> +			err = netdev_nl_queue_dump_one(netdev, skb, info,
> +						       &rxq_idx, &txq_idx);
> +		else
> +			err = -ENODEV;
> +	} else {
> +		for_each_netdev_dump(net, netdev, ctx->ifindex) {
> +			err = netdev_nl_queue_dump_one(netdev, skb, info,
> +						       &rxq_idx, &txq_idx);
> +

unnecessary new line

> +			if (err < 0)
> +				break;
> +			if (!err) {

it only returns 0 or negative errno, doesn't it?

> +				rxq_idx = 0;
> +				txq_idx = 0;
> +			}
> +		}
> +	}
> +	rtnl_unlock();
> +
> +	if (err != -EMSGSIZE)
> +		return err;
> +
> +	ctx->rxq_idx = rxq_idx;
> +	ctx->txq_idx = txq_idx;
> +	return skb->len;
>  }
>  
>  static int netdev_genl_netdevice_event(struct notifier_block *nb,
>
Nambiar, Amritha Oct. 11, 2023, 11:54 p.m. UTC | #2 
On 10/10/2023 7:25 PM, Jakub Kicinski wrote:
> On Fri, 06 Oct 2023 02:14:59 -0700 Amritha Nambiar wrote:
>> +static int
>> +netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
>> +			 u32 q_idx, u32 q_type, const struct genl_info *info)
>> +{
>> +	struct netdev_rx_queue *rxq;
>> +	struct netdev_queue *txq;
>> +	void *hdr;
>> +
>> +	hdr = genlmsg_iput(rsp, info);
>> +	if (!hdr)
>> +		return -EMSGSIZE;
>> +
>> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_ID, q_idx))
>> +		goto nla_put_failure;
>> +
>> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_TYPE, q_type))
>> +		goto nla_put_failure;
>> +
>> +	if (nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
>> +		goto nla_put_failure;
> 
> You can combine these ifs in a single one using ||
> 
Okay, will fix in v5.

>> +	switch (q_type) {
>> +	case NETDEV_QUEUE_TYPE_RX:
>> +		rxq = __netif_get_rx_queue(netdev, q_idx);
>> +		if (rxq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> 
>> +static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
>> +				    u32 q_type)
>> +{
>> +	switch (q_type) {
>> +	case NETDEV_QUEUE_TYPE_RX:
>> +		if (q_id >= netdev->real_num_rx_queues)
>> +			return -EINVAL;
>> +		return 0;
>> +	case NETDEV_QUEUE_TYPE_TX:
>> +		if (q_id >= netdev->real_num_tx_queues)
>> +			return -EINVAL;
>> +		return 0;
>> +	default:
>> +		return -EOPNOTSUPP;
> 
> Doesn't the netlink policy prevent this already?

For this, should I be using "checks: max:" as an attribute property for 
the 'queue-id' attribute in the yaml. If so, not sure how I can give a 
custom value (not hard-coded) for max.
Or should I include a pre-doit hook.

> 
>> +	}
>> +}
> 
>>   int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
>>   {
>> -	return -EOPNOTSUPP;
>> +	u32 q_id, q_type, ifindex;
>> +	struct net_device *netdev;
>> +	struct sk_buff *rsp;
>> +	int err;
>> +
>> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_ID))
>> +		return -EINVAL;
>> +
>> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_TYPE))
>> +		return -EINVAL;
>> +
>> +	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
>> +		return -EINVAL;
> 
> You can combine these checks in a single if using ||

Will fix in v5.

> 
>> +	q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_ID]);
>> +
>> +	q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_TYPE]);
>> +
>> +	ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> 
> No need for the empty lines between these.

Will fix.

> 
>> +	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> +	if (!rsp)
>> +		return -ENOMEM;
>> +
>> +	rtnl_lock();
>> +
>> +	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
>> +	if (netdev)
>> +		err  = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> 
> double space after =

Will fix.

> 
>> +	else
>> +		err = -ENODEV;
>> +
>> +	rtnl_unlock();
>> +
>> +	if (err)
>> +		goto err_free_msg;
>> +
>> +	return genlmsg_reply(rsp, info);
>> +
>> +err_free_msg:
>> +	nlmsg_free(rsp);
>> +	return err;
>> +}
>> +
>> +static int
>> +netdev_nl_queue_dump_one(struct net_device *netdev, struct sk_buff *rsp,
>> +			 const struct genl_info *info, unsigned int *start_rx,
>> +			 unsigned int *start_tx)
>> +{
> 
> Hm. Not sure why you don't operate directly on ctx here.
> Why pass the indexes by pointer individually?
> 

Makes sense. Will fix in v5.

>> +	int err = 0;
>> +	int i;
>> +
>> +	for (i = *start_rx; i < netdev->real_num_rx_queues;) {
>> +		err = netdev_nl_queue_fill_one(rsp, netdev, i,
>> +					       NETDEV_QUEUE_TYPE_RX, info);
>> +		if (err)
>> +			goto out_err;
> 
> return, no need to goto if all it does is returns

Will fix.

> 
>> +		*start_rx = i++;
>> +	}
>> +	for (i = *start_tx; i < netdev->real_num_tx_queues;) {
>> +		err = netdev_nl_queue_fill_one(rsp, netdev, i,
>> +					       NETDEV_QUEUE_TYPE_TX, info);
>> +		if (err)
>> +			goto out_err;
>> +		*start_tx = i++;
>> +	}
>> +out_err:
>> +	return err;
>>   }
>>   
>>   int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
>>   {
>> -	return -EOPNOTSUPP;
>> +	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
>> +	const struct genl_info *info = genl_info_dump(cb);
>> +	struct net *net = sock_net(skb->sk);
>> +	unsigned int rxq_idx = ctx->rxq_idx;
>> +	unsigned int txq_idx = ctx->txq_idx;
>> +	struct net_device *netdev;
>> +	u32 ifindex = 0;
>> +	int err = 0;
>> +
>> +	if (info->attrs[NETDEV_A_QUEUE_IFINDEX])
>> +		ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
>> +
>> +	rtnl_lock();
>> +	if (ifindex) {
>> +		netdev = __dev_get_by_index(net, ifindex);
>> +		if (netdev)
>> +			err = netdev_nl_queue_dump_one(netdev, skb, info,
>> +						       &rxq_idx, &txq_idx);
>> +		else
>> +			err = -ENODEV;
>> +	} else {
>> +		for_each_netdev_dump(net, netdev, ctx->ifindex) {
>> +			err = netdev_nl_queue_dump_one(netdev, skb, info,
>> +						       &rxq_idx, &txq_idx);
>> +
> 
> unnecessary new line

Will fix.

> 
>> +			if (err < 0)
>> +				break;
>> +			if (!err) {
> 
> it only returns 0 or negative errno, doesn't it?
> 
Will fix in v5.

>> +				rxq_idx = 0;
>> +				txq_idx = 0;
>> +			}
>> +		}
>> +	}
>> +	rtnl_unlock();
>> +
>> +	if (err != -EMSGSIZE)
>> +		return err;
>> +
>> +	ctx->rxq_idx = rxq_idx;
>> +	ctx->txq_idx = txq_idx;
>> +	return skb->len;
>>   }
>>   
>>   static int netdev_genl_netdevice_event(struct notifier_block *nb,
>>
> 
>
Jakub Kicinski Oct. 12, 2023, 11:48 p.m. UTC | #3 
On Wed, 11 Oct 2023 16:54:00 -0700 Nambiar, Amritha wrote:
> >> +static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
> >> +				    u32 q_type)
> >> +{
> >> +	switch (q_type) {
> >> +	case NETDEV_QUEUE_TYPE_RX:
> >> +		if (q_id >= netdev->real_num_rx_queues)
> >> +			return -EINVAL;
> >> +		return 0;
> >> +	case NETDEV_QUEUE_TYPE_TX:
> >> +		if (q_id >= netdev->real_num_tx_queues)
> >> +			return -EINVAL;
> >> +		return 0;
> >> +	default:
> >> +		return -EOPNOTSUPP;  
> > 
> > Doesn't the netlink policy prevent this already?  
> 
> For this, should I be using "checks: max:" as an attribute property for 
> the 'queue-id' attribute in the yaml. If so, not sure how I can give a 
> custom value (not hard-coded) for max.
> Or should I include a pre-doit hook.

Weird, I was convinced that if you specify enum by default the code gen
will automatically generate the range check to make sure that the value
is within the enum..

Looks like it does, this is the policy, right?

+static const struct nla_policy netdev_queue_get_do_nl_policy[NETDEV_A_QUEUE_QUEUE_TYPE + 1] = {
+	[NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+	[NETDEV_A_QUEUE_QUEUE_TYPE] = NLA_POLICY_MAX(NLA_U32, 1),
 	                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Nambiar, Amritha Oct. 13, 2023, 12:24 a.m. UTC | #4 
On 10/12/2023 4:48 PM, Jakub Kicinski wrote:
> On Wed, 11 Oct 2023 16:54:00 -0700 Nambiar, Amritha wrote:
>>>> +static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
>>>> +				    u32 q_type)
>>>> +{
>>>> +	switch (q_type) {
>>>> +	case NETDEV_QUEUE_TYPE_RX:
>>>> +		if (q_id >= netdev->real_num_rx_queues)
>>>> +			return -EINVAL;
>>>> +		return 0;
>>>> +	case NETDEV_QUEUE_TYPE_TX:
>>>> +		if (q_id >= netdev->real_num_tx_queues)
>>>> +			return -EINVAL;
>>>> +		return 0;
>>>> +	default:
>>>> +		return -EOPNOTSUPP;
>>>
>>> Doesn't the netlink policy prevent this already?
>>
>> For this, should I be using "checks: max:" as an attribute property for
>> the 'queue-id' attribute in the yaml. If so, not sure how I can give a
>> custom value (not hard-coded) for max.
>> Or should I include a pre-doit hook.
> 
> Weird, I was convinced that if you specify enum by default the code gen
> will automatically generate the range check to make sure that the value
> is within the enum..
> 
> Looks like it does, this is the policy, right?
> 
> +static const struct nla_policy netdev_queue_get_do_nl_policy[NETDEV_A_QUEUE_QUEUE_TYPE + 1] = {
> +	[NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
> +	[NETDEV_A_QUEUE_QUEUE_TYPE] = NLA_POLICY_MAX(NLA_U32, 1),
>   	                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Yes, this is the policy. This policy does validate that the max value 
for 'queue-type' is within the enum range.

I was thinking your review comment was for the entire 
'netdev_nl_queue_validate' function (i.e. if the max queue-id validation 
can be handled in the policy as a range with max value for queue-id, and 
since max queue-id was not a constant, but varies within the kernel, ex: 
netdev->real_num_rx_queues, I was unsure of it...). So, another option I 
could come up with for the validation was a 'pre_doit' hook instead of 
netdev_nl_queue_validate().

If your comment referred to the enum queue-type range alone, I see, 
since the policy handles the max check for queue-type, I can remove the 
default case returning EOPNOTSUPP. Correct me if I'm wrong.
Jakub Kicinski Oct. 13, 2023, 12:36 a.m. UTC | #5 
On Thu, 12 Oct 2023 17:24:41 -0700 Nambiar, Amritha wrote:
> I was thinking your review comment was for the entire 
> 'netdev_nl_queue_validate' function (i.e. if the max queue-id validation 
> can be handled in the policy as a range with max value for queue-id, and 
> since max queue-id was not a constant, but varies within the kernel, ex: 
> netdev->real_num_rx_queues, I was unsure of it...). So, another option I 
> could come up with for the validation was a 'pre_doit' hook instead of 
> netdev_nl_queue_validate().

real_num can change if we're not holding rtnl_lock, and we can't hold
the lock in pre :(

> If your comment referred to the enum queue-type range alone, I see, 
> since the policy handles the max check for queue-type, I can remove the 
> default case returning EOPNOTSUPP. Correct me if I'm wrong.

Yup! I only meant the type, you can trust netlink to validate the type.
Nambiar, Amritha Oct. 13, 2023, 12:40 a.m. UTC | #6 
On 10/12/2023 5:36 PM, Jakub Kicinski wrote:
> On Thu, 12 Oct 2023 17:24:41 -0700 Nambiar, Amritha wrote:
>> I was thinking your review comment was for the entire
>> 'netdev_nl_queue_validate' function (i.e. if the max queue-id validation
>> can be handled in the policy as a range with max value for queue-id, and
>> since max queue-id was not a constant, but varies within the kernel, ex:
>> netdev->real_num_rx_queues, I was unsure of it...). So, another option I
>> could come up with for the validation was a 'pre_doit' hook instead of
>> netdev_nl_queue_validate().
> 
> real_num can change if we're not holding rtnl_lock, and we can't hold
> the lock in pre :(
> 
I see.

>> If your comment referred to the enum queue-type range alone, I see,
>> since the policy handles the max check for queue-type, I can remove the
>> default case returning EOPNOTSUPP. Correct me if I'm wrong.
> 
> Yup! I only meant the type, you can trust netlink to validate the type.

Got it. Thanks!
Jakub Kicinski Oct. 13, 2023, 12:44 a.m. UTC | #7 
On Thu, 12 Oct 2023 17:36:36 -0700 Jakub Kicinski wrote:
> we can't hold the lock in pre :(

To be clear - I meant to say that for dumps we can't hold the lock
across all the callbacks, for do it would technically be fine.
But I don't think that it's better.
diff mbox series

Patch

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 336c608e6a6b..8e8eddf68018 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -6,9 +6,23 @@ 
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/xdp.h>
+#include <net/netdev_rx_queue.h>
 
 #include "netdev-genl-gen.h"
 
+struct netdev_nl_dump_ctx {
+	unsigned long	ifindex;
+	unsigned int	rxq_idx;
+	unsigned int	txq_idx;
+};
+
+static struct netdev_nl_dump_ctx *netdev_dump_ctx(struct netlink_callback *cb)
+{
+	NL_ASSERT_DUMP_CTX_FITS(struct netdev_nl_dump_ctx);
+
+	return (struct netdev_nl_dump_ctx *)cb->ctx;
+}
+
 static int
 netdev_nl_dev_fill(struct net_device *netdev, struct sk_buff *rsp,
 		   const struct genl_info *info)
@@ -111,12 +125,13 @@  int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info)
 
 int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
 	struct net *net = sock_net(skb->sk);
 	struct net_device *netdev;
 	int err = 0;
 
 	rtnl_lock();
-	for_each_netdev_dump(net, netdev, cb->args[0]) {
+	for_each_netdev_dump(net, netdev, ctx->ifindex) {
 		err = netdev_nl_dev_fill(netdev, skb, genl_info_dump(cb));
 		if (err < 0)
 			break;
@@ -129,14 +144,200 @@  int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int
+netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
+			 u32 q_idx, u32 q_type, const struct genl_info *info)
+{
+	struct netdev_rx_queue *rxq;
+	struct netdev_queue *txq;
+	void *hdr;
+
+	hdr = genlmsg_iput(rsp, info);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_ID, q_idx))
+		goto nla_put_failure;
+
+	if (nla_put_u32(rsp, NETDEV_A_QUEUE_QUEUE_TYPE, q_type))
+		goto nla_put_failure;
+
+	if (nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
+		goto nla_put_failure;
+
+	switch (q_type) {
+	case NETDEV_QUEUE_TYPE_RX:
+		rxq = __netif_get_rx_queue(netdev, q_idx);
+		if (rxq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+					     rxq->napi->napi_id))
+			goto nla_put_failure;
+		break;
+	case NETDEV_QUEUE_TYPE_TX:
+		txq = netdev_get_tx_queue(netdev, q_idx);
+		if (txq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+					     txq->napi->napi_id))
+			goto nla_put_failure;
+
+		if (nla_put_u32(rsp, NETDEV_A_QUEUE_TX_MAXRATE,
+				txq->tx_maxrate))
+			goto nla_put_failure;
+		break;
+	}
+
+	genlmsg_end(rsp, hdr);
+
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(rsp, hdr);
+	return -EMSGSIZE;
+}
+
+static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
+				    u32 q_type)
+{
+	switch (q_type) {
+	case NETDEV_QUEUE_TYPE_RX:
+		if (q_id >= netdev->real_num_rx_queues)
+			return -EINVAL;
+		return 0;
+	case NETDEV_QUEUE_TYPE_TX:
+		if (q_id >= netdev->real_num_tx_queues)
+			return -EINVAL;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+netdev_nl_queue_fill(struct sk_buff *rsp, struct net_device *netdev, u32 q_idx,
+		     u32 q_type, const struct genl_info *info)
+{
+	int err;
+
+	err = netdev_nl_queue_validate(netdev, q_idx, q_type);
+	if (err)
+		return err;
+
+	return netdev_nl_queue_fill_one(rsp, netdev, q_idx, q_type, info);
+}
+
 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return -EOPNOTSUPP;
+	u32 q_id, q_type, ifindex;
+	struct net_device *netdev;
+	struct sk_buff *rsp;
+	int err;
+
+	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_ID))
+		return -EINVAL;
+
+	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_TYPE))
+		return -EINVAL;
+
+	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
+		return -EINVAL;
+
+	q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_ID]);
+
+	q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_TYPE]);
+
+	ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
+
+	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!rsp)
+		return -ENOMEM;
+
+	rtnl_lock();
+
+	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
+	if (netdev)
+		err  = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
+	else
+		err = -ENODEV;
+
+	rtnl_unlock();
+
+	if (err)
+		goto err_free_msg;
+
+	return genlmsg_reply(rsp, info);
+
+err_free_msg:
+	nlmsg_free(rsp);
+	return err;
+}
+
+static int
+netdev_nl_queue_dump_one(struct net_device *netdev, struct sk_buff *rsp,
+			 const struct genl_info *info, unsigned int *start_rx,
+			 unsigned int *start_tx)
+{
+	int err = 0;
+	int i;
+
+	for (i = *start_rx; i < netdev->real_num_rx_queues;) {
+		err = netdev_nl_queue_fill_one(rsp, netdev, i,
+					       NETDEV_QUEUE_TYPE_RX, info);
+		if (err)
+			goto out_err;
+		*start_rx = i++;
+	}
+	for (i = *start_tx; i < netdev->real_num_tx_queues;) {
+		err = netdev_nl_queue_fill_one(rsp, netdev, i,
+					       NETDEV_QUEUE_TYPE_TX, info);
+		if (err)
+			goto out_err;
+		*start_tx = i++;
+	}
+out_err:
+	return err;
 }
 
 int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	return -EOPNOTSUPP;
+	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
+	const struct genl_info *info = genl_info_dump(cb);
+	struct net *net = sock_net(skb->sk);
+	unsigned int rxq_idx = ctx->rxq_idx;
+	unsigned int txq_idx = ctx->txq_idx;
+	struct net_device *netdev;
+	u32 ifindex = 0;
+	int err = 0;
+
+	if (info->attrs[NETDEV_A_QUEUE_IFINDEX])
+		ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
+
+	rtnl_lock();
+	if (ifindex) {
+		netdev = __dev_get_by_index(net, ifindex);
+		if (netdev)
+			err = netdev_nl_queue_dump_one(netdev, skb, info,
+						       &rxq_idx, &txq_idx);
+		else
+			err = -ENODEV;
+	} else {
+		for_each_netdev_dump(net, netdev, ctx->ifindex) {
+			err = netdev_nl_queue_dump_one(netdev, skb, info,
+						       &rxq_idx, &txq_idx);
+
+			if (err < 0)
+				break;
+			if (!err) {
+				rxq_idx = 0;
+				txq_idx = 0;
+			}
+		}
+	}
+	rtnl_unlock();
+
+	if (err != -EMSGSIZE)
+		return err;
+
+	ctx->rxq_idx = rxq_idx;
+	ctx->txq_idx = txq_idx;
+	return skb->len;
 }
 
 static int netdev_genl_netdevice_event(struct notifier_block *nb,