Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <1f836577-5f33-4662-bb32-26f99f737ee6@intel.com>
Date:   Mon, 20 Nov 2023 12:40:41 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next] net: hsr: Add support for MC filtering at the
 slave device
Content-Language: en-US
To:     Ravi Gunasekaran <r-gunasekaran@ti.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>
CC:     <bigeasy@linutronix.de>, <horms@kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20231120110105.18416-1-r-gunasekaran@ti.com>
From:   Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <20231120110105.18416-1-r-gunasekaran@ti.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TlJIR2tMTVBmekJzbjJJNk1lcXNLU1VLdmJjc1d6Z24zWTRKVmVjdTVVRGZO?=
 =?utf-8?B?Wk9vM0I5ZHJyc3U4NXpzRkZIbFFHU1lUWU9mR2t5YVZEOXZLL2JhU2lqdnZC?=
 =?utf-8?B?L3pkeWh1S0pWTGtoQkY0NHpvWTg4NU5tNWl5dkF4cWJzY3RhWFptOUNqeHlk?=
 =?utf-8?B?c3Q0bEZZanFXWEtOQUtPSUpmU3pqU01DQkQzQUhkcHhkaDZtWmxxa3JUK28w?=
 =?utf-8?B?MWlBOGRmSGVMR21ja3FWbWtYZTRFWHhjdWFKdVQ2aGJvZmFOSjA1c1crNUxa?=
 =?utf-8?B?a2VVYllBMWN0OWhXZDREK21vN2J3TXdLOHBQcHNnYXAwUzVWVjRSVlhUbkxU?=
 =?utf-8?B?bWJEeHUyU3lBaEU0cDExTGJLSWNWUWNvaVhSSkdIUk1ZZ2pyTXBlS3hYenZE?=
 =?utf-8?B?MHB0NHRES01Uait6TEZ6K0psNVcwQ2YwckVOYXFFSko4c2hiUGZWM3pCWkFV?=
 =?utf-8?B?c2cvbTQybDhjSGJaU1VQNGtjSWx3Nmw0K3NwU1FzM3p5OTNocVFKL3ZIdzN1?=
 =?utf-8?B?T1h2eS9nK3k5b2QwUHc3L2hNRFZNVG5vV3hpbGdGdmRDbGZ6UGFLYTRCN0k1?=
 =?utf-8?B?RVh6b2F2OXI5WXF2K0RpS21jU1N1blM1a05pc1dmUzFQQmN1SzdMZTYzdnhh?=
 =?utf-8?B?Q3FjNnRTUmJTRWw5SHFSTU41RTlkK3ltL2RHVWxSUHhKTHI0OHN1ZDdJZ2RO?=
 =?utf-8?B?cCtTa3ZneWdCR0FnNDUxcEJacXdVQUluU2ViZzRocllnOFg0aGJReVg2ZXNF?=
 =?utf-8?B?b3dTR0E5Vk1yNVY3dlp1b3VTT3J1dEhaSjE5NWp5QVFJQnR5WE9xQWFIKzNm?=
 =?utf-8?B?R0NJR3BQaFU4VmNpcWlXd3dpY0VrQ2V0THJUUkJIdEloZjZ3NjhWczlEeUkv?=
 =?utf-8?B?YWtjZWFtODI5TEptRnc5MFMrRVlDbVI5Z28rR1NEQm5ZWUZXQ3luWlFuem4v?=
 =?utf-8?B?RU1qcnBLbzZncnpiY1RwVi9lMVAzVWhHU0VyWFArQUN4WC9rZmNsdjdJSmRI?=
 =?utf-8?B?dm9PQ1U1YVdPRlB0QXo3cy9salk1VDlPRFhJWnRIT2srUFkzaGZMNEdnNE1E?=
 =?utf-8?B?N3RQbTJNam8rUkJ2aWc1ZVFRWjBFY1FHVExrWDFKMTAvcytPbWdJd3lSc0Js?=
 =?utf-8?B?a3FZU2wvWFlxbitPSFdrZE5IMUc5d0YxWU53NU1HL0VteUkvd0JRMU4rblRo?=
 =?utf-8?B?L0ZJdk5mR1JMY0FkcG5GcjZjZm45anRROXNtTnFIaG9JMTIxOWRZdTE2OU1z?=
 =?utf-8?B?RXhXMWJCbXBEdTJ6UTNGUWtLVVdRb2ZYUzFPTURMQkpJd2JWcVZMQUpqeUJa?=
 =?utf-8?B?ZzMwbWcza0RFOEY4WnM1UVlUQlZTTEtPUllSdXdFL2tYTnhPeEN1bjlVdUE3?=
 =?utf-8?B?VnVrckpqbVBBYy8yUnVWV3YzTjZVWmwzb3IrSjlxaUk5b0lWTytOb3JLQlVa?=
 =?utf-8?B?V1dDRGpwVVlaQVBib21nenhjbGRyb05hODQ4K0hRaG5jUXg1RWU3NkFBUjAz?=
 =?utf-8?B?SnJHYU9VNmNLTS9wRENhVVNRUTEzcGtLblhKaE1FZFlwRnQrLzAzZ0xkSFFC?=
 =?utf-8?B?NHM2T1NnRzlYLzM5TjlRalBaV01oUzEvUm5ONDFBRWJ3QVREeWF1ZzFwbmdG?=
 =?utf-8?B?K0JIUTZDTXdNeWg5N1dSUStSVGNiMndORWswckY2RlJCT0R2R1VhM0dHUlV6?=
 =?utf-8?B?MXkwSEhlU0pNU0Vva3hHSmJ3SlBDVFFtQ3ZaejVxN2FTRHZSM2RrL1pmeVox?=
 =?utf-8?B?T3Njek93RkxSTEJ6cGx4NE1WREpDQnFzTEhwYkJsbEZUMWRGZldMeFd2MzVz?=
 =?utf-8?B?aUZScWh5OHl2V1pYWk9mY3laNkFUa0FsRHRMVXVRMVNTMlZvVlNLeDR5SVBK?=
 =?utf-8?B?em5EYUhnS2lGQnpiUjBoeTV0ZmtpRXgzKzNNbFozazVUTmxMNFFldVFqVkhQ?=
 =?utf-8?B?WjB6ZkFxLytMYlREaGF3blphdVFaTlQ2UngxaFZ3VTJobnBPVUNPOHlWSFl2?=
 =?utf-8?B?QkVOMXI5ZStvNE5zU3BhZFlvTkJlY2dXMmZrOUpmZXp2VzJxYmtnK3hUVUtX?=
 =?utf-8?B?b29jUXJBRzJUQTRNUHE4djlnTkRlcnRWZ2JRVmlIMXl5Q05IbWZEZWdYb0d0?=
 =?utf-8?B?UGFudDY0dnVtU0pVZEd3VTA4bFBLS29sbXRwSDEwVkUyQnkrTnZnOFY2amd6?=
 =?utf-8?B?TkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: eedcfa5f-8a74-41a7-270b-08dbe9bd89e7
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Nov 2023 11:40:47.7299
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Gklt7Da9VzrYlcNgfDUJL3cf0aKwbzrL7LFFGcyyODBssEXomvvZYIdtF9fbTHZHl8tQ8vrN8iecNqYdEkoHH8ILCQw68hcRif1/cWR8okY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5197
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 20 Nov 2023 03:41:10 -0800 (PST)


On 20.11.2023 12:01, Ravi Gunasekaran wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
> 
> When MC (multicast) list is updated by the networking layer due to a
> user command and as well as when allmulti flag is set, it needs to be
> passed to the enslaved Ethernet devices. This patch allows this
> to happen by implementing ndo_change_rx_flags() and ndo_set_rx_mode()
> API calls that in turns pass it to the slave devices using
> existing API calls.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
> ---

One small nit, besides that:
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  net/hsr/hsr_device.c | 67 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 306f942c3b28..4e8f4a3cefbf 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -173,7 +173,24 @@ static int hsr_dev_open(struct net_device *dev)
>  
>  static int hsr_dev_close(struct net_device *dev)
>  {
> -	/* Nothing to do here. */
> +	struct hsr_port *port;
> +	struct hsr_priv *hsr;
> +
> +	hsr = netdev_priv(dev);
> +	hsr_for_each_port(hsr, port) {
> +		if (port->type == HSR_PT_MASTER)
> +			continue;
> +		switch (port->type) {
> +		case HSR_PT_SLAVE_A:
> +		case HSR_PT_SLAVE_B:
> +			dev_uc_unsync(port->dev, dev);
> +			dev_mc_unsync(port->dev, dev);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -404,12 +421,60 @@ void hsr_del_ports(struct hsr_priv *hsr)
>  		hsr_del_port(port);
>  }
>  
> +static void hsr_ndo_set_rx_mode(struct net_device *dev)

I wouldn't add "ndo" to the callback name, let's not stand out from other ndo callbacks here.

> +{
> +	struct hsr_port *port;
> +	struct hsr_priv *hsr;
> +
> +	hsr = netdev_priv(dev);
> +
> +	hsr_for_each_port(hsr, port) {
> +		if (port->type == HSR_PT_MASTER)
> +			continue;
> +		switch (port->type) {
> +		case HSR_PT_SLAVE_A:
> +		case HSR_PT_SLAVE_B:
> +			dev_mc_sync_multiple(port->dev, dev);
> +			dev_uc_sync_multiple(port->dev, dev);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +static void hsr_change_rx_flags(struct net_device *dev, int change)
> +{
> +	struct hsr_port *port;
> +	struct hsr_priv *hsr;
> +
> +	hsr = netdev_priv(dev);
> +
> +	hsr_for_each_port(hsr, port) {
> +		if (port->type == HSR_PT_MASTER)
> +			continue;
> +		switch (port->type) {
> +		case HSR_PT_SLAVE_A:
> +		case HSR_PT_SLAVE_B:
> +			if (change & IFF_ALLMULTI)
> +				dev_set_allmulti(port->dev,
> +						 dev->flags &
> +						 IFF_ALLMULTI ? 1 : -1);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  static const struct net_device_ops hsr_device_ops = {
>  	.ndo_change_mtu = hsr_dev_change_mtu,
>  	.ndo_open = hsr_dev_open,
>  	.ndo_stop = hsr_dev_close,
>  	.ndo_start_xmit = hsr_dev_xmit,
> +	.ndo_change_rx_flags = hsr_change_rx_flags,
>  	.ndo_fix_features = hsr_fix_features,
> +	.ndo_set_rx_mode = hsr_ndo_set_rx_mode,
>  };
>  
>  static struct device_type hsr_type = {
> 
> base-commit: 5a82d69d48c82e89aef44483d2a129f869f3506a