FWIW, I recently hit a bug during a demo that I would have caught but for lack of padding by pg in UT. In my case I had tested full spread of packet sizes using the UT framework, but hadn't done it thoroughly enough with real interfaces and so got bit by b->current_length > iplen with small ip packets.
Unless there are real world scenarios where the device pg is emulating returns < 60b frames I think it would be good just to fix pg to pad. If code breaks based on this, wouldn't that code also break when used in production? :) Thanks, Chris. > On Oct 29, 2019, at 9:01 AM, Paul Vinciguerra <pvi...@vinciconsulting.com> > wrote: > > Hi Ben. > > Isn't it amazing how timing is everything. Would you believe this issue came > up in discussion yesterday? I was suggesting the use of alternate interfaces, > such as over tap/veth/af_packet over pg in tests. > > If the behavior is changed, it should probably be done in a backward > compatible way (maybe an allow-runts flag?). Would it be enough for vpp to > log a counter when it encounters an improperly padded packet? The tests > could then identify the condition and act accordingly. > > It was a topic of discussion because I too am seeing issues and saw a comment > about pg not being an ethernet device. I am seeing/working through the cdp > and the dhcp_client tests passing, but failing and/or erroring in real life. > > > Paul > > > On Tue, Oct 29, 2019 at 6:40 AM Benoit Ganne (bganne) via Lists.Fd.Io > <bganne=cisco....@lists.fd.io> wrote: > Hi all, > > While working on Address Sanitizer, I realized pg pcap replay was not padding > runt frames to 60-bytes before handing them to ethernet-input. With a real > NIC, we should never get packets smaller than 60-bytes. > This means we process very short Ethernet frames in make test. It is very > common, because scapy does not pad either so any packet we build with scapy > will only contains the bytes we define (eg. our process for learning remote > hosts MAC address in make test is generating only Ethernet headers - > 14-bytes). > This cause 2 issues : > 1) we do not catch bugs where we check that payload len advertised by > protocols == received packet payload length. This is not very common but it > happens > 2) it makes make test non-deterministic: there is a lot of places (esp. in > L2 code) where we assume it is safe to load & test values in the 1st > 60-bytes. The assumption is correct but with current pg behavior it means we > branch based on uninitialized values, so 2 identical make test runs could > trigger different codepath > > Does anyone see a drawback to change pg to pad pcap replay frames with 0 up > to 60-bytes? > > Best > ben > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > > View/Reply Online (#14375): https://lists.fd.io/g/vpp-dev/message/14375 > Mute This Topic: https://lists.fd.io/mt/39414067/1594641 > Group Owner: vpp-dev+ow...@lists.fd.io > Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [pvi...@vinciconsulting.com] > -=-=-=-=-=-=-=-=-=-=-=- > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > > View/Reply Online (#14376): https://lists.fd.io/g/vpp-dev/message/14376 > Mute This Topic: https://lists.fd.io/mt/39414067/1826170 > Group Owner: vpp-dev+ow...@lists.fd.io > Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [cho...@chopps.org] > -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#14377): https://lists.fd.io/g/vpp-dev/message/14377 Mute This Topic: https://lists.fd.io/mt/39414067/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
