All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Jeff Johnson <quic_jjohnson@quicinc.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Michael Braun <michael-dev@fami-braun.de>
Cc: Harsh Kumar Bijlani <hbijlani@qti.qualcomm.com>,
	Kalyan Tallapragada <ktallapr@qti.qualcomm.com>,
	Jyothi Chukkapalli <jchukkap@qti.qualcomm.com>,
	Anirban Sirkhell <anirban@qti.qualcomm.com>,
	Johannes Berg <johannes.berg@intel.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, ath12k@lists.infradead.org,
	Jeff Johnson <quic_jjohnson@quicinc.com>
Subject: Re: [PATCH] wifi: mac80211: Fix ieee80211_convert_to_unicast() logic
Date: Fri, 16 Aug 2024 13:31:01 +0200	[thread overview]
Message-ID: <877ccgd7re.fsf@toke.dk> (raw)
In-Reply-To: <20240815-ieee80211_convert_to_unicast-v1-1-648f0c195474@quicinc.com>

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> The current logic in ieee80211_convert_to_unicast() uses skb_clone()
> to obtain an skb for each individual destination of a multicast
> frame, and then updates the destination address in the cloned skb's
> data buffer before placing that skb on the provided queue.
>
> This logic is flawed since skb_clone() shares the same data buffer
> with the original and the cloned skb, and hence each time the
> destination address is updated, it overwrites the previous destination
> address in this shared buffer. As a result, due to the special handing
> of the first valid destination, all of the skbs will eventually be
> sent to that first destination.

Did you actually observe this happen in practice? ieee80211_change_da()
does an skb_ensure_writable() check on the Ethernet header before
writing it, so AFAICT it does not, in fact, overwrite the data of the
original frame.

> Fix this issue by using skb_copy() instead of skb_clone(). This will
> result in a duplicate data buffer being allocated for each
> destination, and hence each skb will be transmitted to the proper
> destination.

Cf the above, it seems this change will just lead to more needless
copying.

-Toke



  reply	other threads:[~2024-08-16 11:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-15 16:18 [PATCH] wifi: mac80211: Fix ieee80211_convert_to_unicast() logic Jeff Johnson
2024-08-16 11:31 ` Toke Høiland-Jørgensen [this message]
2024-08-16 14:30   ` Jeff Johnson
2024-08-16 17:54     ` Felix Fietkau
2024-08-16 20:04       ` Jeff Johnson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877ccgd7re.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=anirban@qti.qualcomm.com \
    --cc=ath12k@lists.infradead.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hbijlani@qti.qualcomm.com \
    --cc=jchukkap@qti.qualcomm.com \
    --cc=johannes.berg@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=ktallapr@qti.qualcomm.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=michael-dev@fami-braun.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=quic_jjohnson@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.