On Sat, Jun 13, 2026 at 5:10 PM Jakub Kicinski [off-list ref] wrote:

On Sat, 13 Jun 2026 17:02:39 -0700 Xiang Mei wrote:

quoted

quoted

quoted

      if (data_len) {
-             int nla_len = nla_total_size(data_len);
              struct nlattr *nla;

-             nla = skb_put(nl_skb, nla_len);
-             nla->nla_type = PSAMPLE_ATTR_DATA;
-             nla->nla_len = nla_attr_size(data_len);
+             nla = nla_reserve(nl_skb, PSAMPLE_ATTR_DATA, data_len);
+             if (!nla)
+                     goto error;

              if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
                      goto error;

Let me know if the new patch makes sense.

I assumed the author intentionally was avoiding the memset for
the memory we will override with data. Otherwise the whole dance
could be avoided and nla_put() would have been the answer.

The reason nla_put() isn't used here is that the payload source is the
sampled skb, which can be nonlinear, so the data has to be gathered with
skb_copy_bits() rather than a flat memcpy().

That too.

quoted

There is no nla_put() variant that takes an skb source, hence the
reserve-then-copy.

But that doesn't require open-coding the attribute: nla_reserve() only
memsets the alignment padding (nla_padlen), never the data region, so
nla_reserve() + skb_copy_bits() writes every byte exactly once with no
redundant memset over the payload. The bug was that the open-coded
version dropped that padding-zero step.

I find it hard to believe that nla_reserve() was not considered.
It's a widely used function in the networking stack.

Thanks for your time of discussion. v3 has been sent.

Xiang

Please move on.