Hi Pravin,
Thanks for the feedback.
>> @@ -4084,10 +4089,13 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
>> *packets_,
>> int i;
>>
>> if (!may_steal) {
>> - dp_packet_batch_clone(&tnl_pkt, packets_);
>> - packets_ = &tnl_pkt;
>> + dp_packet_batch_clone(&tnl_pkt, packets_);
>> + packets_ = &tnl_pkt;
>> }
>>
>> + dp_packet_batch_cutlen(packets_);
>> + dp_packet_batch_reset_cutlen(orig_packets_);
>> +
> is there any need to call dp_packet_batch_reset_cutlen() function
> after dp_packet_batch_cutlen()?
Yes, if may_steal == false, we need to clear the cutlen at the
original packets so that it won't leak to other actions.
>> @@ -4107,22 +4115,41 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
>> *packets_,
>>
>> case OVS_ACTION_ATTR_USERSPACE:
>> if (!fat_rwlock_tryrdlock(&dp->upcall_rwlock)) {
>> + struct dp_packet_bat
>> @@ -1585,6 +1585,12 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
>>
>> for (i = 0; i < cnt; i++) {
>> int size = dp_packet_size(pkts[i]);
>> + uint32_t cutlen = dp_packet_get_cutlen(pkts[i]);
>> +
>> + if (cutlen > 0) {
>> + size -= cutlen;
>> + dp_packet_set_size(pkts[i], size);
>> + }
> No need to check if cutlen is positive value, subtracting zero does
> not matter anyways.
Yes, no need to check.
I did this check on purpose since for dpdk, dp_packet_set_size() does
two assignment, set data_len and pkt_len. If cutlen is rarely used, I
feel it's more efficient adding this check.
>> @@ -717,6 +717,10 @@ (struct netdev *netdev, int qid, struct dp_packet_batch
>> *batch,
>> if (!error) {
>> COVERAGE_INC(netdev_sent);
>> }
>> +
>> + if (!may_steal) {
>> + dp_packet_batch_reset_cutlen(batch);
>> + }
> This could be moved to if block above. it is not required if there is any
> error.
Yes, thanks.
>> diff --git a/tests/odp.at b/tests/odp.at
>> index 7b94c92..e630855 100644
>> --- a/tests/odp.at
>> +++ b/tests/odp.at
> ...
>
>> AT_SETUP([conntrack - controller])
>> CHECK_CONNTRACK()
>> OVS_TRAFFIC_VSWITCHD_START()
>> --
>
> I really like number of tests you have added. Can you also add test to
> validate SLOW_ACTION case of truncate? You can add ovs-appctl command
> to disable datapath truncate support to simulate the case.
Sure, I will do it.
Regards,
William
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
