> The PCAP_SNAPLEN of 1024 surprises me. Wouldn't you want at least > 1500, so that you can capture full Ethernet frames? (Maybe 1514 or > 1518? I don't know whether the snaplen includes the Ethernet and VLAN > headers.)
Yes, this appears to be an oversight. 1518 is probably reasonable. > In netdev_bsd_listen(), does pcap_open_live() usefully set errno? > Perhaps we should log the error text that it reports when it fails. Yes, it looks like we need to VLOG_ERR errbuf and choose a generic errno (EINVAL?) to return from netdev_bsd_listen(). > It's a little sad that the pcap architecture means that we have to do > an extra copy of the packet. Yes. With more significant changes to the overall netdev datapath we could eventually eliminate these copies, but for now we're hoping for a minimally invasive change set. > My docs for libpcap mention a "pcap_next_ex" function that can be used > to distinguish errors from no available packets. That might be easier > to use than pcap_dispatch(). Yes, this looks like a sensible change. Any objection to having that come in as a subsequent commit? The current patch set is largely Gaetano's effort against 1.1.0pre2 with updates from Giuseppe and me to bring it to the current git version. > It would be a good idea to update netdev_bsd_send() to fix the issue > mentioned by the comment: > > /* XXX should support sending even if 'ethertype' was > NETDEV_ETH_TYPE_NONE. > */ > > because according to commit 76c308b50d (netdev-linux: Support 'send' > for netdevs opened with NETDEV_ETH_TYPE_NONE.), which fixed the same > issue in netdev-linux, bonding will not work without this issue being > fixed. I'm not sure if Giuseppe has had an opportunity to look at this yet, but I will otherwise. Thanks again for the review and feedback! Regards, Ed _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
