From: Pu Lehui pulehui@huawei.com
Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory accounting [0]. The commit cannot be independently backported to the 5.10 stable branch, otherwise the related memory when creating devmap will be unrestricted and the associated bpf selftest map_ptr will fail. Let's roll back to rlimit-based memory accounting mode for devmap and re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") to the 5.10 stable branch.
Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0] Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps") Signed-off-by: Pu Lehui pulehui@huawei.com --- kernel/bpf/devmap.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c index 07b5edb2c70f..7eb1282edc8e 100644 --- a/kernel/bpf/devmap.c +++ b/kernel/bpf/devmap.c @@ -109,6 +109,8 @@ static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab, static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr) { u32 valsize = attr->value_size; + u64 cost = 0; + int err;
/* check sanity of attributes. 2 value sizes supported: * 4 bytes: ifindex @@ -136,11 +138,21 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr) return -EINVAL;
dtab->n_buckets = roundup_pow_of_two(dtab->map.max_entries); + cost += (u64) sizeof(struct hlist_head) * dtab->n_buckets; + } else { + cost += (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *); + }
+ /* if map size is larger than memlock limit, reject it */ + err = bpf_map_charge_init(&dtab->map.memory, cost); + if (err) + return -EINVAL; + + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) { dtab->dev_index_head = dev_map_create_hash(dtab->n_buckets, dtab->map.numa_node); if (!dtab->dev_index_head) - return -ENOMEM; + goto free_charge;
spin_lock_init(&dtab->index_lock); } else { @@ -148,10 +160,14 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr) sizeof(struct bpf_dtab_netdev *), dtab->map.numa_node); if (!dtab->netdev_map) - return -ENOMEM; + goto free_charge; }
return 0; + +free_charge: + bpf_map_charge_finish(&dtab->map.memory); + return -ENOMEM; }
static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweiclou...
Please ignore this mail if the patch is not relevant for upstream.
On 2024/9/20 18:37, kernel test robot wrote:
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweiclou...
Please ignore this mail if the patch is not relevant for upstream.
This fix only involves 5.10, other versions are no problem.
On Mon, Sep 23, 2024 at 02:38:07PM +0800, Pu Lehui wrote:
On 2024/9/20 18:37, kernel test robot wrote:
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH 5.10] bpf: Fix mismatch memory accounting for devmap maps Link: https://lore.kernel.org/stable/20240920103950.3931497-1-pulehui%40huaweiclou...
Please ignore this mail if the patch is not relevant for upstream.
This fix only involves 5.10, other versions are no problem.
why? If this is true, then you need to get the relevant bpf maintainers to review and ack it.
thanks,
greg k-h
On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
From: Pu Lehui pulehui@huawei.com
Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory accounting [0]. The commit cannot be independently backported to the 5.10 stable branch, otherwise the related memory when creating devmap will be unrestricted and the associated bpf selftest map_ptr will fail. Let's roll back to rlimit-based memory accounting mode for devmap and re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") to the 5.10 stable branch.
Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0] Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
Should we just revert these changes instead?
thanks,
greg k-h
On 2024/9/27 15:56, Greg Kroah-Hartman wrote:
On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
From: Pu Lehui pulehui@huawei.com
Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory accounting [0]. The commit cannot be independently backported to the 5.10 stable branch, otherwise the related memory when creating devmap will be unrestricted and the associated bpf selftest map_ptr will fail. Let's roll back to rlimit-based memory accounting mode for devmap and re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") to the 5.10 stable branch.
Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0] Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
Should we just revert these changes instead?
Yes, Greg. My patch is to revert these two commits and re-adapt commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches").
Shall we need to split this patch into multiple patches?
thanks,
greg k-h
On Fri, Sep 27, 2024 at 04:03:36PM +0800, Pu Lehui wrote:
On 2024/9/27 15:56, Greg Kroah-Hartman wrote:
On Fri, Sep 20, 2024 at 10:39:50AM +0000, Pu Lehui wrote:
From: Pu Lehui pulehui@huawei.com
Commit 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps") relies on the v5.11+ basic mechanism of memcg-based memory accounting [0]. The commit cannot be independently backported to the 5.10 stable branch, otherwise the related memory when creating devmap will be unrestricted and the associated bpf selftest map_ptr will fail. Let's roll back to rlimit-based memory accounting mode for devmap and re-adapt the commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") to the 5.10 stable branch.
Link: https://lore.kernel.org/bpf/20201201215900.3569844-1-guro@fb.com [0] Fixes: 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches") Fixes: 70294d8bc31f ("bpf: Eliminate rlimit-based memory accounting for devmap maps")
Should we just revert these changes instead?
Yes, Greg. My patch is to revert these two commits and re-adapt commit 225da02acdc9 ("bpf: Fix DEVMAP_HASH overflow check on 32-bit arches").
Shall we need to split this patch into multiple patches?
Yes, please do so.
linux-stable-mirror@lists.linaro.org