From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4921A35CB87 for ; Fri, 12 Jun 2026 12:33:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781267585; cv=none; b=jDvOHLyHac5dxNWhgaOBIbMDx7P7vd1GeX5aWPM1kDxsPpye6WhW6fYGRFv2WW6Ule6YdLIWse46vcjCyRs3mYfmQUZcciXTdlC4vfnJ/hdUjZlkQQSnBYRbjIZjGcIhhlQBx1q/AIInMMEmQfaJQQ9DRwjMQ3zVHuTX5l2nK9c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781267585; c=relaxed/simple; bh=neqF3ZJ0RM58yRx8EhB87dBgrW1cPu5BIRNKi5z4ylk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=XZnBJauo4JDAimfwBh/aBRMPKp/8eUNm4pFljwiDKhwEhg2CcIhtfsGS65eqiok057FlCWGghfsIU4GEUEfDsxp3QIsKZrm/AvrjzrLiZsB+p1O48vVVj9SYLWQ0VotfXJkfZ0TGIwyZxgx1NE3zPJDPcyLul7t6/XgGBR6hhqg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=RW90IJz6; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=DXzM8ZDP; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="RW90IJz6"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="DXzM8ZDP" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1781267583; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2sitiNtJI2vvGtaVwoQZDdJSQ/FtwXIAN0T9g+Ogu2g=; b=RW90IJz60LbXPKPgXydDNFvYuufj0HiTsIBooSueAmuuiQoGvdFSAoVsg/l3Oiczlg65E+ Xv/Ax5H3krgjKasXY2ugWK+oEcHhPoOkgFZmuUGyCcIOO21VGRGgWzYs8W3/+kWD/cdJuc qg84LpPCkkMe/hqDK8Eff0XcqoEyZN4= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-60-6rXSEo87P4m7wJRIO1Jkvw-1; Fri, 12 Jun 2026 08:33:02 -0400 X-MC-Unique: 6rXSEo87P4m7wJRIO1Jkvw-1 X-Mimecast-MFC-AGG-ID: 6rXSEo87P4m7wJRIO1Jkvw_1781267581 Received: by mail-ed1-f70.google.com with SMTP id 4fb4d7f45d1cf-68b6f4ef5a2so823532a12.1 for ; Fri, 12 Jun 2026 05:33:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1781267581; x=1781872381; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=2sitiNtJI2vvGtaVwoQZDdJSQ/FtwXIAN0T9g+Ogu2g=; b=DXzM8ZDPSD+lufhUrO5jfWonNy3ynZnZDzCuGUGWad5WAw5kMSDEQDaoGjf3+Z2dn/ ToW9V/bd2uT4RnD/rmPTMXDvlL2phhTYCyYgtvf9rsRA6+QvQTz12nVQD8hIdk5LMvhK XGZuujUF0xXkkBQTZ+XUQ+/woERt6cn4UvhKt1c4YxTA5yVrv9OGO4EEvM3oKV3t9lM0 YIpnbYz25SglYKxbFwChZLfD/QLmyxLWlGwosW5P5EmHi/yWiopu0mOP9BtfYwi6BjAM dODEgb2x7SH0ccu157iNCpbka7Nab+zmw/C3uwdm4WOwf9G24PNgR5o2rQZgMGtF87BL S0lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781267581; x=1781872381; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=2sitiNtJI2vvGtaVwoQZDdJSQ/FtwXIAN0T9g+Ogu2g=; b=Rio9JxDmLeEpwTYONVnWmY3v5H0QA+haCgXywS4QkiSk7xGDlMRyX0Kouj1npW7/XB 9hmC5AUJgN60ofNNNJQJ1bE8BCwD5cxJK1VhwKOQHh2zurtInnaqSs5eAjjArZJF7Ym6 pSzs8x7L2jBIXIUPX4IsruYCdW9+OOhPhObhrGjcjda51lR9y9cGT4UzY2y/FYvZ2rKC Zoh1fwikY3Wkmmh8LW7WtfQhCsJHb8jjEH05spXoRiUDwh1B7gDDuTigwTMcsLl8Foi7 SHAF0nEJEZKXfxdLoiEgdIcJmFuFvi4cNfKng4udSwbYMnfie/jHLqUZVFI1fGvVT7y2 DswA== X-Forwarded-Encrypted: i=1; AFNElJ/pNawENXNbUzLq+QbGN8OYKRRESG8veeDG9cGHJtuEa/xp/3wsqNhmWEVt2yc2Q8i9Co5MPMKgrS0psSg=@vger.kernel.org X-Gm-Message-State: AOJu0Yx/NjZqaLd0etzK5jUlef/59SyNF6okI7L87Z1V/rdVWzdaUPn7 ap6JIUYzqJWCNBv6EwmPNc0jbSLHCJmtrvG7YPQVkKG+RcE46JL1OJ7jXI22P48RLIsHk/qemUE AGk7d52OVKPOtIOkdKGFvUwWO3ZKM6JlmjG9RkYwR1QISRdmU2yVDQQdxqyNxKdpFaw== X-Gm-Gg: Acq92OGmh5YYsmtaIGF14mWmS+yWd4bUMqLsQmHrD5+7nCf/juM8n/3iMc+b//YrnIM lD62tE0NhHvjzFZzJqQIOH4dOOQuuu7V1bWDXNeFQMGc/NlB8aCaqOpsGoD1wHrzc6FRdEemTLb pAElruhy+JK/EveKmsTxItsor6tJOGqXcVgqzELi9pVH/Qw7Lp37Ok8XesSjP9Tgc9U6DX5NnHd k+V8/OU3IhNbVt67IEYbRlI4PjEzmr74SGKPOeIDmVTVkN3ZT8g39VDCXRtBDqKa1+xZg1LHa+4 KB//NqMg3VaLqU/VP8D6y/k7LvwGhlhXPz1kZDwlpbqQCuqNkYwHvXWzNMCclIcijlN+jhx3bjK fC35Q2mi0P0QRVRqERD9TYzQep7SgixpF5Zh0O/9I0BpyBzjByHJcdBnXogpzy/Du3zDkK+c= X-Received: by 2002:a05:6402:5203:b0:68f:cc95:8c11 with SMTP id 4fb4d7f45d1cf-69378a5ecf3mr1241831a12.28.1781267580686; Fri, 12 Jun 2026 05:33:00 -0700 (PDT) X-Received: by 2002:a05:6402:5203:b0:68f:cc95:8c11 with SMTP id 4fb4d7f45d1cf-69378a5ecf3mr1241809a12.28.1781267580205; Fri, 12 Jun 2026 05:33:00 -0700 (PDT) Received: from [10.44.49.141] (5920ab7b.static.cust.trined.nl. [89.32.171.123]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-6937938a0acsm637062a12.19.2026.06.12.05.32.57 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 12 Jun 2026 05:32:58 -0700 (PDT) From: Eelco Chaudron To: Adrian Moreno Cc: netdev@vger.kernel.org, aconole@redhat.com, pabeni@redhat.com, Ilya Maximets , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Simon Horman , "open list:OPENVSWITCH" , open list Subject: Re: [PATCH net-next v4 1/4] net: openvswitch: make flow_table an rcu pointer Date: Fri, 12 Jun 2026 14:32:54 +0200 X-Mailer: MailMate (2.0r6292) Message-ID: <747F3742-1EEC-47FA-AE5A-FDA6C469C42F@redhat.com> In-Reply-To: <20260611045817.1302665-2-amorenoz@redhat.com> References: <20260611045817.1302665-1-amorenoz@redhat.com> <20260611045817.1302665-2-amorenoz@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On 11 Jun 2026, at 6:58, Adrian Moreno wrote: > This patch turns "flow_table" from being embedded into "datapath" to > being an rcu protected pointer. No functional change intended. > > Signed-off-by: Adrian Moreno Hi Adrian, Thanks for the series. I need some time to take a look, as this is the first time I reviewed this series. I'll probably reply to the other patches early next week. Some small comments below. Cheers, Eelco [...] > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c [...] > @@ -752,12 +759,16 @@ static struct genl_family dp_packet_genl_family _= _ro_after_init =3D { > static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stat= s *stats, > struct ovs_dp_megaflow_stats *mega_stats) > { > + struct flow_table *table =3D ovsl_dereference(dp->table); > int i; > > memset(mega_stats, 0, sizeof(*mega_stats)); > + memset(stats, 0, sizeof(*stats)); > > - stats->n_flows =3D ovs_flow_tbl_count(&dp->table); > - mega_stats->n_masks =3D ovs_flow_tbl_num_masks(&dp->table); > + if (table) { > + stats->n_flows =3D ovs_flow_tbl_count(table); > + mega_stats->n_masks =3D ovs_flow_tbl_num_masks(table); > + } The memset above seems like an overkill, why not do this: if (table) { stats->n_flows =3D ovs_flow_tbl_count(table); mega_stats->n_masks =3D ovs_flow_tbl_num_masks(table); } else { stats->n_flows =3D 0; } > > stats->n_hit =3D stats->n_missed =3D stats->n_lost =3D 0; [...] > @@ -1598,8 +1639,13 @@ static int ovs_dp_cmd_fill_info(struct datapath = *dp, struct sk_buff *skb, > struct ovs_dp_stats dp_stats; > struct ovs_dp_megaflow_stats dp_megaflow_stats; > struct dp_nlsk_pids *pids =3D ovsl_dereference(dp->upcall_portids); > + struct flow_table *table; > int err, pids_len; > > + table =3D ovsl_dereference(dp->table); > + if (!table) > + return -ENODEV; ovs_dp_cmd_fill_info() can now return -ENODEV when table is NULL, but every caller immediately follows with: BUG_ON(err < 0); so this would panic the kernel. I guess you fix this in the 3rd patch, but just mentioning this as patch 1 is unsafe to apply in isolation. > ovs_header =3D genlmsg_put(skb, portid, seq, &dp_datapath_genl_family= , > flags, cmd); > if (!ovs_header) [...] > @@ -1917,7 +1972,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, st= ruct genl_info *info) > /* Called with ovs_mutex. */ > static void __dp_destroy(struct datapath *dp) > { > - struct flow_table *table =3D &dp->table; > + struct flow_table *table =3D ovsl_dereference(dp->table); > int i; > > if (dp->user_features & OVS_DP_F_TC_RECIRC_SHARING) > @@ -1948,6 +2003,7 @@ static void __dp_destroy(struct datapath *dp) > > /* RCU destroy the ports, meters and flow tables. */ > call_rcu(&dp->rcu, destroy_dp_rcu); In theory the table pointer cannot be NULL here, but would it make sense to add a WARN_ON(!table) to catch any future mistakes? > + call_rcu(&table->rcu, ovs_flow_tbl_destroy_rcu); > } > [...] > diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.= c > index 67d5b8c0fe79..3b7518e3394d 100644 > --- a/net/openvswitch/flow_table.c > +++ b/net/openvswitch/flow_table.c > @@ -406,15 +406,19 @@ int ovs_flow_tbl_masks_cache_resize(struct flow_t= able *table, u32 size) > return 0; > } > > -int ovs_flow_tbl_init(struct flow_table *table) > +struct flow_table *ovs_flow_tbl_alloc(void) > { > struct table_instance *ti, *ufid_ti; > + struct flow_table *table; > struct mask_cache *mc; > struct mask_array *ma; > > + table =3D kzalloc_obj(*table, GFP_KERNEL); > + if (!table) > + return ERR_PTR(-ENOMEM); Add an extra blank line? > mc =3D tbl_mask_cache_alloc(MC_DEFAULT_HASH_ENTRIES); > if (!mc) > - return -ENOMEM; > + goto free_table; > > ma =3D tbl_mask_array_alloc(MASK_ARRAY_SIZE_MIN); > if (!ma) > @@ -435,7 +439,7 @@ int ovs_flow_tbl_init(struct flow_table *table) > table->last_rehash =3D jiffies; > table->count =3D 0; > table->ufid_count =3D 0; > - return 0; > + return table; > > free_ti: > __table_instance_destroy(ti); > @@ -443,7 +447,9 @@ int ovs_flow_tbl_init(struct flow_table *table) > __mask_array_destroy(ma); > free_mask_cache: > __mask_cache_destroy(mc); > - return -ENOMEM; > +free_table: > + kfree(table); > + return ERR_PTR(-ENOMEM); > } [...]