Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <ad4c0688-6e4b-bc66-55c3-8d33493f7c8c@redhat.com>
Date:   Thu, 5 May 2022 16:14:25 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.8.1
Subject: Re: [PATCH 3/3] vdpa/mlx5: Add RX MAC VLAN filter support
Content-Language: en-US
To:     Eli Cohen <elic@nvidia.com>, "mst@redhat.com" <mst@redhat.com>
Cc:     "virtualization@lists.linux-foundation.org" 
        <virtualization@lists.linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "si-wei.liu@oracle.com" <si-wei.liu@oracle.com>
References: <20220411122942.225717-1-elic@nvidia.com>
 <20220411122942.225717-4-elic@nvidia.com>
 <eb381955-0a64-011f-2732-943c60541b18@redhat.com>
 <DM8PR12MB540032F367E9DB4C277AB946ABC19@DM8PR12MB5400.namprd12.prod.outlook.com>
From:   Jason Wang <jasowang@redhat.com>
In-Reply-To: <DM8PR12MB540032F367E9DB4C277AB946ABC19@DM8PR12MB5400.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2022/5/2 13:38, Eli Cohen 写道:
>>> +static virtio_net_ctrl_ack handle_ctrl_vlan(struct mlx5_vdpa_dev *mvdev, u8 cmd)
>>> +{
>>> +	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>>> +	virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
>>> +	struct mlx5_control_vq *cvq = &mvdev->cvq;
>>> +	struct virtio_net_ctrl_vlan vlan;
>>> +	size_t read;
>>> +	u16 id;
>>> +
>>> +	switch (cmd) {
>>> +	case VIRTIO_NET_CTRL_VLAN_ADD:
>>> +		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, (void *)&vlan, sizeof(vlan));
>>> +		if (read != sizeof(vlan))
>>> +			break;
>>> +
>>> +		id = mlx5vdpa16_to_cpu(mvdev, vlan.id);
>>> +		if (mac_vlan_add(ndev, ndev->config.mac, id, true))
>>> +			break;
>> This may work now but I wonder if we had the plan to support
>> VIRTIO_NET_F_CTRL_RX?
>>
>> if $mac is not in $mac_table
>>       drop;
>> if $vlan is not in $vlan_table
>>       drop;
>>
>> With that features we probably requires the dedicated vlan filters
>> without a mac? Or do we want to a $mac * $vlans rules?
>>
>> If yes, maybe it's better to decouple vlan filters from mac now.
>>
> If we use dedicated filter tables for VLAN and MAC working in series,
> we may not have full control over incoming traffic filtering.
> For example, suppose we have VLAN table allowing v1 and v2 to go through,
> and a MAC table allowing m1 and m2 to go through.
>
> If we want only (v1, m1) and (v2, m2) to go through but not (v1, m2) or (v2, m1)
> then it would not be possible to block the latter.


Yes, but this is currently how virtio-net work in the spec.


>
> As I can see, the spec does not require that finesse


Yes.


>   but I wonder if this not
> what real life requires.


Then we need to extend the spec.


> If you think we should follow the spec let me know and will prepare a new version.


It should be fine now. (But if we want too support CTRL_RX we need some 
refactoring on the codes).

So:

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


>