| Message ID | 20191021140436.24696-1-gmuthukr@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] lacp: warn transmit failure of lacp pdu | expand |
On Mon, Oct 21, 2019 at 07:34:36PM +0530, Gowrishankar Muthukrishnan wrote: > It might be difficult to trace whether LACP PDU tx (as in > response) was successful when the pdu was not transmitted by > egress slave for various reasons (including resource contention > within NIC) and only way to trace its fate is by looking at > ofproto->stats.tx_[packets/bytes] and slave port stats. > > Adding a warning when there is tx failure could help user > debug at the root of this problem. > > Signed-off-by: Gowrishankar Muthukrishnan <gmuthukr@redhat.com> This logic seems like it's probably true for all users of ofproto_dpif_send_packet(), not just LACP PDU transmission. Would it be a good idea to simply make ofproto_dpif_send_packet() log a warning when tx fails?
Hi Ben, Idea is to log lacp specific tx failure. A one step further as in ofproto_dpif_send_packet has to make the debug info more generic, even though it would cater all of its calling functions. For example, bundle_send_learning_packets reports such tx failures for gratuitous packets. I think, we need to handle such warning at the calling place itself to debug quickly. Thanks, Gowrishankar On Fri, Oct 25, 2019 at 2:47 AM Ben Pfaff <blp@ovn.org> wrote: > On Mon, Oct 21, 2019 at 07:34:36PM +0530, Gowrishankar Muthukrishnan wrote: > > It might be difficult to trace whether LACP PDU tx (as in > > response) was successful when the pdu was not transmitted by > > egress slave for various reasons (including resource contention > > within NIC) and only way to trace its fate is by looking at > > ofproto->stats.tx_[packets/bytes] and slave port stats. > > > > Adding a warning when there is tx failure could help user > > debug at the root of this problem. > > > > Signed-off-by: Gowrishankar Muthukrishnan <gmuthukr@redhat.com> > > This logic seems like it's probably true for all users of > ofproto_dpif_send_packet(), not just LACP PDU transmission. Would it be > a good idea to simply make ofproto_dpif_send_packet() log a warning when > tx fails? >
On Mon, Oct 21, 2019 at 07:34:36PM +0530, Gowrishankar Muthukrishnan wrote: > It might be difficult to trace whether LACP PDU tx (as in > response) was successful when the pdu was not transmitted by > egress slave for various reasons (including resource contention > within NIC) and only way to trace its fate is by looking at > ofproto->stats.tx_[packets/bytes] and slave port stats. > > Adding a warning when there is tx failure could help user > debug at the root of this problem. > > Signed-off-by: Gowrishankar Muthukrishnan <gmuthukr@redhat.com> I applied this to master, thanks!
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 496a16c8a..4af771585 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3405,7 +3405,11 @@ send_pdu_cb(void *port_, const void *pdu, size_t pdu_size) pdu_size); memcpy(packet_pdu, pdu, pdu_size); - ofproto_dpif_send_packet(port, false, &packet); + error = ofproto_dpif_send_packet(port, false, &packet); + if (error) { + VLOG_WARN_RL(&rl, "port %s: cannot transmit LACP PDU (%s).", + port->bundle->name, ovs_strerror(error)); + } dp_packet_uninit(&packet); } else { static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(1, 10);
It might be difficult to trace whether LACP PDU tx (as in response) was successful when the pdu was not transmitted by egress slave for various reasons (including resource contention within NIC) and only way to trace its fate is by looking at ofproto->stats.tx_[packets/bytes] and slave port stats. Adding a warning when there is tx failure could help user debug at the root of this problem. Signed-off-by: Gowrishankar Muthukrishnan <gmuthukr@redhat.com> --- ofproto/ofproto-dpif.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)