Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <6d8e441da60ce29d0007c5a6cf173e0a7a1353f6.camel@sipsolutions.net>
Subject: Re: [RFC 1/7] mac80211: add support for .ndo_fill_forward_path
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Cc:     netdev@vger.kernel.org, pablo@netfilter.org, ryder.lee@mediatek.com
Date:   Tue, 13 Jul 2021 18:46:20 +0200
In-Reply-To: <20210713160745.59707-2-nbd@nbd.name>
References: <20210713160745.59707-1-nbd@nbd.name>
         <20210713160745.59707-2-nbd@nbd.name>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2021-07-13 at 18:07 +0200, Felix Fietkau wrote:
> +
> +static int ieee80211_netdev_fill_forward_path(struct net_device_path_ctx *ctx,
> +					      struct net_device_path *path)
> +{
> +	struct ieee80211_sub_if_data *sdata;
> +	struct ieee80211_local *local;
> +	struct sta_info *sta;
> +	int ret = -ENOENT;
> +
> +	sdata = IEEE80211_DEV_TO_SUB_IF(ctx->dev);
> +	local = sdata->local;
> +
> +	if (!local->ops->net_fill_forward_path)
> +		return -EOPNOTSUPP;
> +
> +	rcu_read_lock();
> +	switch (sdata->vif.type) {
> +	case NL80211_IFTYPE_AP_VLAN:
> +		sta = rcu_dereference(sdata->u.vlan.sta);
> +		if (sta)
> +			break;
> +		if (!sdata->wdev.use_4addr)
> +			goto out;

Am I confusing things, or is this condition inverted? If it's not 4-addr
then you won't have a u.vlan.sta, but you might still want to look up
the station more generally, no?

> +		fallthrough;
> +	case NL80211_IFTYPE_AP:
> +		if (is_multicast_ether_addr(ctx->daddr))
> +			goto out;
> +		sta = sta_info_get_bss(sdata, ctx->daddr);

Or maybe this shouldn't use _bss() here, but then you'd need to write a
sta_info_get() also in the VLAN case, no?

Which might actually be better or even correct, because if the station
is on the VLAN you probably *don't* want to find it here if the
interface that's being passed is the AP, no?

johannes