Bill Fischofer(Bill-Fischofer-Linaro) 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:
At this point, since we're tagging Tiger Moth RC1 this week, all of api-next is
being merged into master, so this will be on that base.
> Juha-Matti Tilli(jmtilli) wrote:
> Not at least on master; it isn't deprecated there. Because my changes weren't
> an API change, I based the code on master instead of basing it on api-next.
> Of course I'm willing to support this patch by implementing the
> non-deprecated API as well, if this is ever merged to master and then via
> master to api-next.
>> Juha-Matti Tilli(jmtilli) wrote:
>> 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_r158602550
updated_at 2017-12-24 13:47:07
