Re: [PATCH net v2] net: ipa: properly limit modem routing table use

From: Jakub Kicinski
Date: Tue Sep 20 2022 - 11:14:10 EST


On Tue, 13 Sep 2022 15:46:02 -0500 Alex Elder wrote:
> IPA can route packets between IPA-connected entities. The AP and
> modem are currently the only such entities supported, and no routing
> is required to transfer packets between them.
>
> The number of entries in each routing table is fixed, and defined at
> initialization time. Some of these entries are designated for use
> by the modem, and the rest are available for the AP to use. The AP
> sends a QMI message to the modem which describes (among other
> things) information about routing table memory available for the
> modem to use.
>
> Currently the QMI initialization packet gives wrong information in
> its description of routing tables. What *should* be supplied is the
> maximum index that the modem can use for the routing table memory
> located at a given location. The current code instead supplies the
> total *number* of routing table entries. Furthermore, the modem is
> granted the entire table, not just the subset it's supposed to use.
>
> This patch fixes this. First, the ipa_mem_bounds structure is
> generalized so its "end" field can be interpreted either as a final
> byte offset, or a final array index. Second, the IPv4 and IPv6
> (non-hashed and hashed) table information fields in the QMI
> ipa_init_modem_driver_req structure are changed to be ipa_mem_bounds
> rather than ipa_mem_array structures. Third, we set the "end" value
> for each routing table to be the last index, rather than setting the
> "count" to be the number of indices. Finally, instead of allowing
> the modem to use all of a routing table's memory, it is limited to
> just the portion meant to be used by the modem. In all versions of
> IPA currently supported, that is IPA_ROUTE_MODEM_COUNT (8) entries.
>
> Update a few comments for clarity.
>
> Fixes: 530f9216a9537 ("soc: qcom: ipa: AP/modem communications")
> Signed-off-by: Alex Elder <elder@xxxxxxxxxx>
> ---
> v2: Update the element info arrays defining the modified QMI message
> so it uses the ipa_mem_bounds_ei structure rather than the
> ipa_mem_array_ei structure. The message format is identical,
> but the code was incorrect without that change.

Unclear to me why, ipa_mem_bounds_ei and ipa_mem_array_ei seem
identical, other than s/end/count/. Overall the patch feels
a touch too verbose for a fix, makes it harder to grasp the key
functional change IMHO. I could be misunderstanding but please
keep the goal of making fixes small and crisp in mind for the future.