Re: [PATCH net-next 11/18] ionic: Add Rx filter and rx_mode nod support

[Shannon Nelson <snelson@pensando.io>]

On 6/25/19 4:44 PM, Jakub Kicinski wrote:
> On Thu, 20 Jun 2019 13:24:17 -0700, Shannon Nelson wrote:
>> Add the Rx filtering and rx_mode NDO callbacks.  Also add
>> the deferred work thread handling needed to manage the filter
>> requests otuside of the netif_addr_lock spinlock.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>>   static int ionic_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto,
>>   				  u16 vid)
>>   {
>> -	netdev_info(netdev, "%s: stubbed\n", __func__);
>> +	struct lif *lif = netdev_priv(netdev);
>> +	struct ionic_admin_ctx ctx = {
>> +		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
>> +		.cmd.rx_filter_del = {
>> +			.opcode = CMD_OPCODE_RX_FILTER_DEL,
>> +			.lif_index = cpu_to_le16(lif->index),
>> +		},
>> +	};
>> +	struct rx_filter *f;
>> +	int err;
>> +
>> +	spin_lock_bh(&lif->rx_filters.lock);
>> +
>> +	f = ionic_rx_filter_by_vlan(lif, vid);
>> +	if (!f) {
>> +		spin_unlock_bh(&lif->rx_filters.lock);
>> +		return -ENOENT;
>> +	}
>> +
>> +	netdev_dbg(netdev, "rx_filter del VLAN %d (id %d)\n", vid,
>> +		   le32_to_cpu(ctx.cmd.rx_filter_del.filter_id));
>> +
>> +	ctx.cmd.rx_filter_del.filter_id = cpu_to_le32(f->filter_id);
>> +	ionic_rx_filter_free(lif, f);
>> +	spin_unlock_bh(&lif->rx_filters.lock);
>> +
>> +	err = ionic_adminq_post_wait(lif, &ctx);
>> +	if (err)
>> +		return err;
>>
>>   	return 0;
> nit: return directly?
Sure.

>
>>   }
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> index 8129fa20695a..c3ecf1df9c2c 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> @@ -60,6 +60,29 @@ struct qcq {
>>   #define napi_to_qcq(napi)	container_of(napi, struct qcq, napi)
>>   #define napi_to_cq(napi)	(&napi_to_qcq(napi)->cq)
>>   
>> +enum deferred_work_type {
>> +	DW_TYPE_RX_MODE,
>> +	DW_TYPE_RX_ADDR_ADD,
>> +	DW_TYPE_RX_ADDR_DEL,
>> +	DW_TYPE_LINK_STATUS,
>> +	DW_TYPE_LIF_RESET,
>> +};
>> +
>> +struct deferred_work {
> If you don't mind prefixing these structures with ionic_ that'd be
> great.  I'm worried deferred_work is too close to delayed_work..

Yes.

>
>> +	struct list_head list;
>> +	enum deferred_work_type type;
>> +	union {
>> +		unsigned int rx_mode;
>> +		u8 addr[ETH_ALEN];
>> +	};
>> +};
>> +
>> +struct deferred {
>> +	spinlock_t lock;		/* lock for deferred work list */
>> +	struct list_head list;
>> +	struct work_struct work;
>> +};