On 1/24/2025 1:59 PM, Balamurugan Mahalingam wrote:
> On 1/24/2025 3:45 AM, Nicolas Escande wrote:
>> On Thu Jan 9, 2025 at 7:49 PM CET, Balamurugan Mahalingam wrote:
>> [...]
>>> diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c
>>> index 76626d925adf..9d05fe7a870c 100644
>>> --- a/drivers/net/wireless/ath/ath12k/dp_tx.c
>>> +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c
>>> @@ -217,7 +217,7 @@ static int ath12k_dp_tx_align_payload(struct ath12k_base *ab,
>>> }
>>>
>>> int ath12k_dp_tx(struct ath12k *ar, struct ath12k_link_vif *arvif,
>>> - struct sk_buff *skb)
>>> + struct sk_buff *skb, bool gsn_valid, int mcbc_gsn)
>>> {
>>> struct ath12k_base *ab = ar->ab;
>>> struct ath12k_dp *dp = &ab->dp;
>>> @@ -290,13 +290,24 @@ int ath12k_dp_tx(struct ath12k *ar, struct ath12k_link_vif *arvif,
>>> msdu_ext_desc = true;
>>> }
>>>
>>> + if (gsn_valid) {
>>> + ti.meta_data_flags =
>>> + u32_encode_bits(HTT_TCL_META_DATA_TYPE_GLOBAL_SEQ_NUM,
>>> + HTT_TCL_META_DATA_TYPE) |
>>> + u32_encode_bits(mcbc_gsn, HTT_TCL_META_DATA_GLOBAL_SEQ_NUM);
>>> + }
>>> +
>> Hello,
>>
>> As ti is zeroed initilised isn't safer to do 'ti.meta_data_flags |='
>> instead of 'ti.meta_data_flags ='. This way if we add another flag in
>> meta_data_flag in the same func for another reason, we will have less risk of
>> overriding the said flags ?
>>
>
> Hello Nicolas,
> In this case, this is an intentional reset of any previous residual
> value in ti.meta_data_flags field.
> The meta_data_flags bit definition is different depending upon the
> META_DATA_TYPE bit's value.
So in this case there can never be any conflicting information that came from
the assignment:
ti.meta_data_flags = arvif->tcl_metadata;
??
Perhaps add a comment that states you are intentionally overriding the
information that came from arvif->tcl_metadata?
/jeff
>
>>> ti.encap_type = ath12k_dp_tx_get_encap_type(arvif, skb);
>>> ti.addr_search_flags = arvif->hal_addr_search_flags;
>>> ti.search_type = arvif->search_type;
>>> ti.type = HAL_TCL_DESC_TYPE_BUFFER;
>>> ti.pkt_offset = 0;
>>> ti.lmac_id = ar->lmac_id;
>>> - ti.vdev_id = arvif->vdev_id;
>>> + if (gsn_valid)
>>> + ti.vdev_id = arvif->vdev_id +
>>> + HTT_TX_MLO_MCAST_HOST_REINJECT_BASE_VDEV_ID;
>>> + else
>>> + ti.vdev_id = arvif->vdev_id;
>>> ti.bss_ast_hash = arvif->ast_hash;
>>> ti.bss_ast_idx = arvif->ast_idx;
>>> ti.dscp_tid_tbl_idx = 0;
>
