[dpdk-dev] [PATCH 07/21] vhost: add iotlb helper functions
Maxime Coquelin
maxime.coquelin at redhat.com
Fri Sep 8 11:28:57 CEST 2017
On 09/08/2017 11:21 AM, Yuanhan Liu wrote:
> On Fri, Sep 08, 2017 at 10:50:49AM +0200, Maxime Coquelin wrote:
>>>>>> +{
>>>>>> + struct vhost_iotlb_entry *node, *temp_node;
>>>>>> +
>>>>>> + rte_rwlock_write_lock(&vq->iotlb_lock);
>>>>>> +
>>>>>> + TAILQ_FOREACH_SAFE(node, &vq->iotlb_list, next, temp_node) {
>>>>>> + TAILQ_REMOVE(&vq->iotlb_list, node, next);
>>>>>> + rte_mempool_put(vq->iotlb_pool, node);
>>>>>> + }
>>>>>> +
>>>>>> + rte_rwlock_write_unlock(&vq->iotlb_lock);
>>>>>> +}
>>>>>> +
>>>>>> +void vhost_user_iotlb_cache_insert(struct vhost_virtqueue *vq, uint64_t iova,
>>>>>> + uint64_t uaddr, uint64_t size, uint8_t perm)
>>>>>> +{
>>>>>> + struct vhost_iotlb_entry *node, *new_node;
>>>>>> + int ret;
>>>>>> +
>>>>>> + ret = rte_mempool_get(vq->iotlb_pool, (void **)&new_node);
>>>>>> + if (ret) {
>>>>>> + RTE_LOG(ERR, VHOST_CONFIG, "IOTLB pool empty, invalidate cache\n");
>>>>>
>>>>> It's a cache, why not considering remove one to get space for new one?
>>>>
>>>> It would mean having to track every lookups not to remove hot entries,
>>>> which would have an impact on performance.
>>>
>>> You were removing all caches, how can we do worse than that? Even a
>>> random evict would be better. Or, more simply, just to remove the
>>> head or the tail?
>>
>> I think removing head or tail could cause deadlocks.
>> For example it needs to translate from 0x0 to 0x2000, with page size
>> being 0x1000.
>>
>> If cache is full, when inserting 0x1000-0x1fff, it will remove
>> 0x0-0xfff, so a miss will be sent for 0x0-0xffff and 0x1000-0x1fff will
>> be remove at insert time, etc...
>
> Okay, that means we can't simply remove the head or tail.
>
>> Note that we really need to size the cache large enough for performance
>> purpose, so having the cache full could be cause by either buggy or
>> malicious guest.
>
> I agree. But for the malicious guest, it could lead to a DDOS attack:
> assume it keeps vhost running out of cache and then vhost keeps printing
> above message.
>
> What I suggested was to evict one (by some polices) to get a space for
> the new one we want to insert. Note that it won't be a performance issue,
> IMO, due to we only do that when we run out of caches, which, according
> to your sayings, should not happen in normal cases.
Ok, so let's randomly remove one entry. What about using something like:
rte_rand() % IOTLB_CACHE_SIZE
>
> --yliu
>
>>>> Moreover, the idea is to have the cache large enough, else you could
>>>> face packet drops due to random cache misses.
>>>>
>>>> We might consider to improve it, but I consider it an optimization that
>>>> could be implemented later if needed.
>>>>
>>>>>> + vhost_user_iotlb_cache_remove_all(vq);
>>>>>> + ret = rte_mempool_get(vq->iotlb_pool, (void **)&new_node);
>>>>>> + if (ret) {
>>>>>> + RTE_LOG(ERR, VHOST_CONFIG, "IOTLB pool still empty, failure\n");
>>>>>> + return;
>>>>>> + }
>>>>>> + }
More information about the dev
mailing list