On Wed, Nov 6, 2024 at 3:17 PM Paul Szczepanek wrote: > > On 30/10/2024 17:08, Nicholas Pratte wrote: > > There exists a bug within i40e NICs in which the async sniffer does not > > catch send packets as a result of the callback function sending packets > > too quickly before the NICs are ready to start capturing. > > > > There could be a multitude of reasons why this happens on these NICs, but > > for the time being, inserting a one second delay in the callback function > > will suffice. > > I can confirm the issue exists but we should explore a more definitive > solution than adding a wait. Ideally instead of relying on the callback > to send packets we should verify readines elsewhere in our sniffer and > send packets when ready in our framework and not as part of the scapy > sniffer constructor. > From looking at the documentation, it is the case that the standard way of verifying readiness for the asyncsniffer is via the started_callback arg in the asyncsniffer constructor. You can see some similar discussion here: https://github.com/secdev/scapy/issues/3208 So, if this is standard, it is probably best to remain within this framework. I have been messing with this series tonight and although I still can't tell why started_callback isn't calling on true sniffer readiness, I think Nick's time.sleep calls are okay. I will say, the modification of duration in this series is odd to me. It looks like the _shell_start_and_stop_sniffing function arg has no default, and no value is passed in in the call coming from send_packets_and_capture. My preference would be to provide a default to the duration arg (say, 1) and remove the arbitrary "duration + 1" in this series. I also believe the comments about i40e should be removed. We understand that this series is adding a delay to support sniffer readiness, but we don't know why this behavior was originally seen on an i40e NIC, and whether it's isolated to that driver. Perhaps there would be a way to loop polling of scapy for sniffer readiness, but I don't see how this would be better or different than the asyncsniffer callback arg (which is essentially the same according to docs). So, in my view the best thing to do is for me to fix up the commit per my comments (and any others anyone has) and apply this.