Received-SPF: pass (google.com: domain of linux-kernel+bounces-22274-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <367b1642-c09a-4bc6-ac63-7692b716174d@virtuozzo.com>
Date: Wed, 10 Jan 2024 21:48:15 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/4] netfilter: nfnetlink_log: use proper helper for
 fetching physinif
Content-Language: en-US
To: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org, kernel@openvz.org
References: <20240110110451.5473-1-ptikhomirov@virtuozzo.com>
 <20240110110451.5473-2-ptikhomirov@virtuozzo.com>
 <20240110133333.GA24888@breakpoint.cc>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
In-Reply-To: <20240110133333.GA24888@breakpoint.cc>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?WWRFY1hBV3VZTUFkT29pQmZlYm9UUy8yZW1FOGNJOVFSbmVRaEpPSkJrODhD?=
 =?utf-8?B?aUpZYUxBbGVGbzhvU1d0bTRMNFZaT3F2RnNqZW1GL3BiY0hjdUJlblpWcitY?=
 =?utf-8?B?V1RaY0ZhWWFPQWlCM2VJQmwwbVVOQWVvZXc0V2xPOWxwSFdlNytuRWdmbFIr?=
 =?utf-8?B?MmxqNE0rb1NXTVBtNVMraWM1Nkoxa3pzU0NPcmtqcGhYNW1jWFMyZ3hjS00x?=
 =?utf-8?B?eWwrNkRBVlNqOUhoTkxsNTFOTlQ1cWcxZGUxMTA2U1R0NWgyK0wxODY3NHFJ?=
 =?utf-8?B?NDZPVzV3Y0VxZXF6a09HQlhGU3UyWG5EUVphU2lkY0E5aDdJVk1PbTRmNW94?=
 =?utf-8?B?REplQTlCZnhNR3pDM2UveEo1MnBkamxINTI1ODJXRlMwSERJdUEwWGRGMlVo?=
 =?utf-8?B?UXRUVGhlaUdsTUFBZ2ltVnpjT3RsWVhuVlFhSzVNaFBwS1FUMkRmeDNXNG9v?=
 =?utf-8?B?Mmx3dU1mVkcrWnpoL2dmbERVZmQ4MlFaMUJYM1ZjbENIZGx0dnNVWFd3eFRl?=
 =?utf-8?B?Q0N2M1VmSnJiUlBRYTdyWGJwZ1IwR2lYN1VadWlCdkRTS29ZWmFYR2Z5YWRN?=
 =?utf-8?B?M1pxU01aUndjZ2hGaWh2aHlrQ1luSlRGaWUzV1JjSDNIYU56RW9mU1FsMzZG?=
 =?utf-8?B?NDhhYW5DbHVDU2RIOEk1VTdNSGZ6ZFdYd3loSjZJVCtSMVU0cVAvRWlBd3Rw?=
 =?utf-8?B?KzhDN3RCSzJKMkwxYU9VRzUxVkZCQ0ZSSzBjOUxWRHRlbTM4RFJ5NzdYbEZl?=
 =?utf-8?B?SXR0RUlKcnZaMGd4MG1WVXBkaVFCQVRWVld2bWltWDB4Z2M1Y0RJaTN2b3BC?=
 =?utf-8?B?YWtaOHI0VzM1dW9WV0pIcHVOa3ErSFAzUTlDVFlIbkR5NSsrdUtxaWlLenlH?=
 =?utf-8?B?RkFadmgxVUV0N3pKV3NOYTFFMjQwSEFWNmY3WDZObGxGNkhWTkhVSEJXZU5Y?=
 =?utf-8?B?TER6c3Y1ZEFNU1A4eDdaTEdaZEwyZnhOd2xCVklGbG4zdWdBdHBlNVk4Q2dt?=
 =?utf-8?B?Z0NzQk1icW02N3hteVFOQnlQN1RnajFnZ0hMajRpN1NSN3FrUE03NHVwZUZK?=
 =?utf-8?B?VC9HVmQ3U0hkbXBLS29IU2NaT29vUzduTWlWc3lpV2c2b1hkZWhXZWJaZjF1?=
 =?utf-8?B?NkQwTXMwbGxHZ3ZvVGsyTk5KZFp3RzFBaGUySHA5bngvWk9vMm5hN1hlMlNi?=
 =?utf-8?B?RmR1ZGhPZG5ndDJ4OWNYZGFkYmtWaWJTaHNiVlFrbW8xT0lvd0VYSW1MUTY4?=
 =?utf-8?B?emhnNWs1WVhoSFFzTktVNEI4N2pZVjByZDg4U29ZOXVPaU1ZajNVTUx5K3Fy?=
 =?utf-8?B?TjB5L3ZwbHlJOHJBbGVuUUQvZGVPb3lwSWxCK0RHOXZLLzNaZnZ0RlhDVmM4?=
 =?utf-8?B?eCt5WG5GS0RGb2pSQ3lOZDRHVEZFT0YvK2xkYW5IQ3MvZy9SNFpDVlBuV0tp?=
 =?utf-8?B?NWo2enBvc1EwOXpCUzlyNVRGVUZMRUxFV3VLbFM4YTNaWDlBdmNqNmFKOER2?=
 =?utf-8?B?RHVJbUlmdE1nYWxEUkcybVU2S0JGajJkNXIweHpObjlyTlltQkVnbDI5VTBM?=
 =?utf-8?B?a2M3aDdBVURZcGZweDJaVFlMbjZNSVhicEYwS2tJaTc5UnVPMUUwZUVRM2lD?=
 =?utf-8?B?Z3lHK0NZbDZLWGpzVmNpcm0wUnM5MWZ6cHZpaVFmbExUaFJMTTdCcVM1SU1V?=
 =?utf-8?B?SGN0bFZsb0lDTDh4dlM5RTIvUW91UklHWnlDcU93SFNIYjd6OWRlbVY3anpt?=
 =?utf-8?B?T3U3cDFPenNQd2FmNG5VeGZhNEFRZXl3M2pzVUZ4WlYxU3kxR2o5WWZtUXRa?=
 =?utf-8?B?VURYWFhIQVJ5YW5mK3l4NGxOaEdLR1AxcTVBL0xpVmZyZUtYUU9JVE91WFMy?=
 =?utf-8?B?Y2ZpS2hoSHNaYVdRNzJBc2Q4a2t0b01DM3ROQlIvVWZEMmo5dTMxMXdzVXpX?=
 =?utf-8?B?cGZpZ1h3WjRxSzRmdU5Rd293Z01VcDJwdGxuYVVsN0FmNEJrdy9FeERIT1J6?=
 =?utf-8?B?NEJiTVlwYmI2dFRtWkZjU0h1cU5jZE92NWI5ZURTYUgyaC84YmRsRDllbENL?=
 =?utf-8?B?Y3NFZUJXVlVJci9RTzBJTHRJZHYzcm1iVmlKVmJGaURQTVhZSDZWZ1lnblJt?=
 =?utf-8?B?b0xpeDZXSE5ENFdGaG9RaDhEK3hXUWE0eTc3V3lmTjdZSDV5UmNMWE9BQWpR?=
 =?utf-8?B?UkdMdFR1Qk41ZFlWOVFhTE93blNVZ3RjZzJua2VYS1pqU3RvaDZGSXN0dVBI?=
 =?utf-8?B?bzVsTXVCbVVnbWNGYnZNeTNoV3dBPT0=?=
X-OriginatorOrg: virtuozzo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a198e0d9-5e56-4092-e27f-08dc11e2cf9b
X-MS-Exchange-CrossTenant-AuthSource: DU0PR08MB9003.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jan 2024 13:48:22.6340
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DawzuBZeAHSqGEotznUkYJFtDBn/pP/AR7kdz9obBgcMOU5ohgvHDmb2FIJtIzouGkN+MfNtkFLShNDkiGI9yrRPDzl1US9lkQQw2LswAfk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR08MB7468


On 10/01/2024 21:33, Florian Westphal wrote:
> Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
>> We don't use physindev in __build_packet_message except for getting
>> physinif from it. So let's switch to nf_bridge_get_physinif to get what
>> we want directly.
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>>   net/netfilter/nfnetlink_log.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
>> index f03f4d4d7d889..134e05d31061e 100644
>> --- a/net/netfilter/nfnetlink_log.c
>> +++ b/net/netfilter/nfnetlink_log.c
>> @@ -508,7 +508,7 @@ __build_packet_message(struct nfnl_log_net *log,
>>   					 htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
>>   				goto nla_put_failure;
>>   		} else {
>> -			struct net_device *physindev;
>> +			int physinif;
>>   
>>   			/* Case 2: indev is bridge group, we need to look for
>>   			 * physical device (when called from ipv4) */
>> @@ -516,10 +516,10 @@ __build_packet_message(struct nfnl_log_net *log,
>>   					 htonl(indev->ifindex)))
>>   				goto nla_put_failure;
>>   
>> -			physindev = nf_bridge_get_physindev(skb);
>> -			if (physindev &&
>> +			physinif = nf_bridge_get_physinif(skb);
>> +			if (physinif &&
>>   			    nla_put_be32(inst->skb, NFULA_IFINDEX_PHYSINDEV,
>> -					 htonl(physindev->ifindex)))
>> +					 htonl(physinif)))
> 
> I think you can drop this patch and make the last patch pass
> nf_bridge_info->physinif directly.

The whole Idea of this patch was to replace nf_bridge_get_physindev with 
nf_bridge_get_physinif before the patch which propagates net, so that we 
don't need to propagate net first and then in later patch remove it when 
replacing with nf_bridge_get_physinif.

But I spoiled it by forgetting to remove net propagation to 
__build_packet_message...

Is it ok if I leave this patch as is, but instead remove:

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 134e05d31061e..ad93dd77e6071 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -463,7 +463,8 @@ __build_packet_message(struct nfnl_log_net *log,
                         const struct net_device *outdev,
                         const char *prefix, unsigned int plen,
                         const struct nfnl_ct_hook *nfnl_ct,
-                       struct nf_conn *ct, enum ip_conntrack_info ctinfo)
+                       struct nf_conn *ct, enum ip_conntrack_info ctinfo,
+                       struct net *net)
  {
         struct nfulnl_msg_packet_hdr pmsg;
         struct nlmsghdr *nlh;
@@ -804,7 +805,7 @@ nfulnl_log_packet(struct net *net,

         __build_packet_message(log, inst, skb, data_len, pf,
                                 hooknum, in, out, prefix, plen,
-                               nfnl_ct, ct, ctinfo);
+                               nfnl_ct, ct, ctinfo, net);

         if (inst->qlen >= qthreshold)
                 __nfulnl_flush(inst);

from third patch?

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.