[PATCH] ath11k: Add support for dynamic vlan

[Seevalamuthu M (QUIC)]

> -----Original Message-----
> From: Seevalamuthu M (QUIC) <quic_seevalam at quicinc.com>
> Sent: Tuesday, February 8, 2022 11:00 AM
> To: Kalle Valo <kvalo at kernel.org>; Seevalamuthu M (QUIC)
> <quic_seevalam at quicinc.com>
> Cc: ath11k at lists.infradead.org; linux-wireless at vger.kernel.org
> Subject: RE: [PATCH] ath11k: Add support for dynamic vlan
> 
> Hi Kalle,
> 
> > -----Original Message-----
> > From: Kalle Valo <kvalo at kernel.org>
> > Sent: Monday, January 17, 2022 6:03 PM
> > To: Seevalamuthu M (QUIC) <quic_seevalam at quicinc.com>
> > Cc: ath11k at lists.infradead.org; linux-wireless at vger.kernel.org
> > Subject: Re: [PATCH] ath11k: Add support for dynamic vlan
> >
> > Seevalamuthu Mariappan <quic_seevalam at quicinc.com> writes:
> >
> > > Advertise AP-VLAN interface type for vlan support in driver.
> > > Metadata information in dp_tx is added to notify firmware that
> > > multicast/broadcast packets are encrypted in software.
> > >
> > > Tested-on: IPQ8074 hw2.0 AHB
> > > WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
> > >
> > > Signed-off-by: Seevalamuthu Mariappan <quic_seevalam at quicinc.com>
> >
> > [...]
> >
> > > --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> > > +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> > > @@ -78,6 +78,44 @@ enum hal_encrypt_type
> > ath11k_dp_tx_get_encrypt_type(u32 cipher)
> > >  	}
> > >  }
> > >
> > > +#define HTT_META_DATA_ALIGNMENT    0x8
> > > +
> > > +static int ath11k_dp_metadata_align_skb(struct sk_buff *skb, u8
> > > +align_len) {
> > > +	if (unlikely(skb_cow_head(skb, align_len)))
> > > +		return -ENOMEM;
> > > +
> > > +	skb_push(skb, align_len);
> > > +	memset(skb->data, 0, align_len);
> > > +	return 0;
> > > +}
> >
> > [...]
> >
> > > @@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct
> > ath11k_vif *arvif,
> > >  		goto fail_remove_idr;
> > >  	}
> > >
> > > +	/* Add metadata for sw encrypted vlan group traffic */
> > > +	if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab-
> > >dev_flags) &&
> > > +	    !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> > > +	    !info->control.hw_key &&
> > > +	    ieee80211_has_protected(hdr->frame_control)) {
> > > +		/* HW requirement is that metadata should always point to a
> > > +		 * 8-byte aligned address. So we add alignment pad to start
> > of
> > > +		 * buffer. HTT Metadata should be ensured to be multiple of
> > 8-bytes
> > > +		 *  to get 8-byte aligned start address along with align_pad
> > added
> > > +		 */
> > > +		align_pad = ((unsigned long)skb->data) &
> > (HTT_META_DATA_ALIGNMENT - 1);
> > > +		ret = ath11k_dp_metadata_align_skb(skb, align_pad);
> > > +		if (unlikely(ret))
> > > +			goto fail_remove_idr;
> >
> > If you push the skb like that shouldn't you also reserve the room for
> > it in struct ieee80211_hw::extra_tx_headroom?
> 
> [seevalam] : already this is handled using skb_cow_head() in
> ath11k_dp_metadata_align_skb(). Please correct me if I misunderstood your
> point.

Hi Kalle, 

Do you have any comments on this? 

Regards,
Seevalamuthu M