Re: [PATCH net-next 01/13] net: sched: Pass qdisc reference in struct flow_block_offload

[Petr Machata <petrm@mellanox.com>]

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> On Fri, Jul 10, 2020 at 04:56:54PM +0300, Ido Schimmel wrote:
>> From: Petr Machata <petrm@mellanox.com>
>>
>> Previously, shared blocks were only relevant for the pseudo-qdiscs ingress
>> and clsact. Recently, a qevent facility was introduced, which allows to
>> bind blocks to well-defined slots of a qdisc instance. RED in particular
>> got two qevents: early_drop and mark. Drivers that wish to offload these
>> blocks will be sent the usual notification, and need to know which qdisc it
>> is related to.
>>
>> To that end, extend flow_block_offload with a "sch" pointer, and initialize
>> as appropriate. This prompts changes in the indirect block facility, which
>> now tracks the scheduler instead of the netdevice. Update signatures of
>> several functions similarly. Deduce the device from the scheduler when
>> necessary.
>>
>> Signed-off-by: Petr Machata <petrm@mellanox.com>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> ---
>>  drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c  | 11 ++++++----
>>  .../ethernet/mellanox/mlx5/core/en/rep/tc.c   | 11 +++++-----
>>  .../net/ethernet/netronome/nfp/flower/main.h  |  2 +-
>>  .../ethernet/netronome/nfp/flower/offload.c   | 11 ++++++----
>>  include/net/flow_offload.h                    |  9 ++++----
>>  net/core/flow_offload.c                       | 12 +++++------
>>  net/netfilter/nf_flow_table_offload.c         | 17 +++++++--------
>>  net/netfilter/nf_tables_offload.c             | 20 ++++++++++--------
>>  net/sched/cls_api.c                           | 21 +++++++++++--------
>>  9 files changed, 63 insertions(+), 51 deletions(-)
>>
> [...]
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
>> index eefeb1cdc2ee..4fc42c1955ff 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
>> @@ -404,7 +404,7 @@ static void mlx5e_rep_indr_block_unbind(void *cb_priv)
>>  static LIST_HEAD(mlx5e_block_cb_list);
>>
>>  static int
>> -mlx5e_rep_indr_setup_block(struct net_device *netdev,
>> +mlx5e_rep_indr_setup_block(struct Qdisc *sch,
>>  			   struct mlx5e_rep_priv *rpriv,
>>  			   struct flow_block_offload *f,
>>  			   flow_setup_cb_t *setup_cb,
>> @@ -412,6 +412,7 @@ mlx5e_rep_indr_setup_block(struct net_device *netdev,
>>  			   void (*cleanup)(struct flow_block_cb *block_cb))
>>  {
>>  	struct mlx5e_priv *priv = netdev_priv(rpriv->netdev);
>> +	struct net_device *netdev = sch->dev_queue->dev;
>
> This break indirect block support for netfilter since the driver
> is assuming a Qdisc object.

Sorry, I don't follow. You mean mlx5 driver? What does it mean to
"assume a qdisc object"?

Is it incorrect to rely on the fact that the netdevice can be deduced
from a qdisc, or that there is always a qdisc associated with a block
binding point?