On Wed, May 01, 2013 at 12:47:06PM +0100, Ian Campbell wrote:

On Wed, 2013-05-01 at 12:40 +0100, Wei Liu wrote:

quoted

On Wed, May 01, 2013 at 12:21:43PM +0100, Ian Campbell wrote:

quoted

On Wed, 2013-05-01 at 11:53 +0100, Wei Liu wrote:

quoted

On Wed, May 01, 2013 at 11:32:41AM +0100, Ian Campbell wrote:

quoted

On Tue, 2013-04-30 at 17:50 +0100, Wei Liu wrote:

quoted

Tune xen_netbk_count_requests to not touch working array beyond limit, so that
we can make working array size constant.

Is this really correct when max_skb_slots > XEN_NETIF_NR_SLOTS_MIN?
Seems like we would either overrun the array or drop frames which
max_skb_slots suggests we should accept?

So the max_skb_slots for now is the standard to determine whether a
guest is malicious, not the maximum slots we can process.

Perhaps I've have misunderstood this patch then but it looks to me like
it will cause us to drop skbs which use slots > XEN_NETIF_NR_SLOTS_MIN
and < max_skb_slots, i.e. ones which are considered non-malicious by the
above definition. Or it will cause us to access indexes into
xen_netbk_tx_build_gops.txfrags which are > XEN_NETIF_NR_SLOTS_MIN.

Any packet using more than XEN_NETIF_NR_SLOTS_MIN are considered
malformed at this point. The behavior is documented in previous commit
log. 2810e5b9a "xen-netback: coalesce slots in TX path and fix
regressions".

"""
The behavior of netback for packet is thus:

        1-18            slots: valid
       19-max_skb_slots slots: drop and respond with an error
       max_skb_slots+   slots: fatal error
"""

OK, so my understanding was wrong and this patch is doing the right
thing.

However it does seem rather like NR_SLOTS_MIN and max_skb_slots are a
bit misnamed. They are actually NR_SLOTS_MAX and fatal_skb_slots? The
NR_SLOTS{MIN/MAX} disparity is particularly confusing in the context of
this code (I understand its the minimum that a backend must support, but
its still confusing in the context of these functions).

Yes probably the naming is weird.

Probably we can do

  #define XEN_NETBK_SLOTS_MAX XEN_NETIF_NR_SLOTS_MIN
  max_skb_slots -> fatal_skb_slots

if it makes things clearer.


Wei.