Juha-Matti Tilli(jmtilli) replied on github web page:

example/l2fwd_simple/odp_l2fwd_simple.c
line 43
@@ -204,6 +206,23 @@ int main(int argc, char **argv)
        global.if1 = create_pktio(argv[optind + 1], pool, &global.if1in,
                                                                &global.if1out);
 
+       /* Do some operations to increase code coverage in tests */
+       if (odp_pktio_mac_addr(global.if0, &correct_src, sizeof(correct_src))
+           != sizeof(correct_src))
+               printf("Warning: can't get MAC address\n");
+       else if (memcmp(&correct_src, &global.src, sizeof(correct_src)) != 0)
+               printf("Warning: src MAC invalid\n");
+
+       odp_pktio_promisc_mode_set(global.if0, true);
+       odp_pktio_promisc_mode_set(global.if1, true);
+       (void)odp_pktio_promisc_mode(global.if0);
+       (void)odp_pktio_promisc_mode(global.if1);
+
+       mtu1 = odp_pktio_mtu(global.if0);
+       mtu2 = odp_pktio_mtu(global.if1);


Comment:
Ah, it is deprecated in api-next. My patch, however, is to master as it has no 
API changes.

> Juha-Matti Tilli(jmtilli) wrote:
> Shouldn't be too hard to support, just needs some space to store the flag. 
> But will do.


>> Juha-Matti Tilli(jmtilli) wrote:
>> In theory, a STATUS 0 would be considered an error here because the expected 
>> status is -1 which is converted to 255.


>>> Juha-Matti Tilli(jmtilli) wrote:
>>> Will fix.


>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>> The problem is that the pktio validation test case doesn't like an 
>>>> interface where packets are not being received. It fails a number of 
>>>> tests, and then for the pktio_test_recv_tmo it just hangs. With the 
>>>> validation tests, an entirely different test for a quiescent interface 
>>>> would be needed. Also, considering that this is testing a particular 
>>>> driver ("null"), not a particular API, I'm not entirely sure that API 
>>>> validation test would be the right location...


>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>> It isn't, at least in the master version on which this branch is based.


>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>> This is taken from the .pcap pktio, and incremented by one to make it 
>>>>>> distinct from the .pcap MAC address. But agree, an internal API to 
>>>>>> generate the MAC address would be great. It needs to be made sure the 
>>>>>> multicast bit is not being set, then.


>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>> This will also be fixed in the next version.


>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>> I'll fix in the next version.


>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>> I'm not entirely sure about this... If the programmer wants to wait 
>>>>>>>>> indefinitely, that's perhaps what we should do, which is indeed what 
>>>>>>>>> is done now. The odp_pktio_stop() comment should perhaps be made to 
>>>>>>>>> the PR #341. A good question is do we really need the empty readfds 
>>>>>>>>> fd_set. Probably we could do without it.


>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>> Will do. This will result in a checkpatch warning then, but if that 
>>>>>>>>>> is not an obstacle to a merge, I can do this.


>>>>>>>>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>>>>>>>>> Ok, the only reason I used this call was to increase code coverage. 
>>>>>>>>>>> But is it deprecated? I grepped through the repository, finding to 
>>>>>>>>>>> evidence that any API having the name mtu would be deprecated...


>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>> This should be supportable also. Just like it is done in other 
>>>>>>>>>>>> software interfaces.


>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>> Maybe we should add internal API to generate random (or 
>>>>>>>>>>>>> semi-random) local MAC-addresses with L bit being set?


>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>> Just use ODP_UNUSED and forget about checkpatch being unoptimal 
>>>>>>>>>>>>>> here. 


>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>> just exit $STATUS. This would allow underlying program to 
>>>>>>>>>>>>>>> return 42 to mark the test as skipped


>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>> this should go under `if test_example` 


>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>> I think this should be now in one of top-level .gitignore 
>>>>>>>>>>>>>>>>> files.


>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>> This should probably go to a separate PR. And ideally 
>>>>>>>>>>>>>>>>>> examples should not add much to code coverage on top of 
>>>>>>>>>>>>>>>>>> tests, so you might want also to add code to api testcases.


>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>> `odp_pktio_mtu()` is a deprecated API and should not be 
>>>>>>>>>>>>>>>>>>> used in new code.


>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>> Where does this "magic number" come from? Why not 
>>>>>>>>>>>>>>>>>>>> `00:00:00:00:00:00`?


>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>> This is a deprecated API. Not sure we need to include it 
>>>>>>>>>>>>>>>>>>>>> for a new Pktio type.


>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>> The parameter should be `num`, not `len` here.


>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>> `odp_packet_free_multi()` would be better here.


>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>> Same comments about `ODP_PKTIN_WAIT` here.


>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> Shouldn't it be an error to try to receive from a 
>>>>>>>>>>>>>>>>>>>>>>>>> null device with `ODP_PKTIN_WAIT` since such a call 
>>>>>>>>>>>>>>>>>>>>>>>>> would never complete? Alternately one could support 
>>>>>>>>>>>>>>>>>>>>>>>>> such semantics and simply wait indefinitely until 
>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_stop()` is called to stop the interface.
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Actually, looking at the semantics of 
>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_stop()` we're not precise about what 
>>>>>>>>>>>>>>>>>>>>>>>>> happens if there are pending `odp_pktio_recv_tmo()` 
>>>>>>>>>>>>>>>>>>>>>>>>> call(s) on the interface at the time 
>>>>>>>>>>>>>>>>>>>>>>>>> `odp_pktio_stop()` is called. Presumably these should 
>>>>>>>>>>>>>>>>>>>>>>>>> be terminated, reporting no packets received when the 
>>>>>>>>>>>>>>>>>>>>>>>>> stop call is made.


>>>>>>>>>>>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> In this case it's probably better to be consistent 
>>>>>>>>>>>>>>>>>>>>>>>>>> and use `(void)pktio_entry` and `(void)index` as 
>>>>>>>>>>>>>>>>>>>>>>>>>> well. Mixing this idiom with `ODP_UNUSED` looks odd. 
>>>>>>>>>>>>>>>>>>>>>>>>>> It also needs no explanation as both are acceptable.


https://github.com/Linaro/odp/pull/365#discussion_r158597918
updated_at 2017-12-24 09:38:55

Reply via email to