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

From: Jakub Kicinski <[email-masked]>
Date: Tue, 20 Sep 2022 08:14:00 -0700

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 <[email-masked]>
> ---
> 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.