From 2fee002031b64717c9932aa28a6d85a1c892d11e Mon Sep 17 00:00:00 2001 From: "Daniele E. Domenichelli" Date: Wed, 17 Jun 2020 08:47:52 +0200 Subject: [PATCH] os/PortCore: Do not set TOS to 0 if DSCP is an unknown string Also improve debugging by adding a few debug messages --- doc/release/master/fix_qos_invalid_dscp.md | 10 +++++ src/libYARP_os/src/yarp/os/impl/PortCore.cpp | 41 +++++++++++++++---- .../src/yarp/os/impl/SocketTwoWayStream.cpp | 1 + 3 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 doc/release/master/fix_qos_invalid_dscp.md diff --git a/doc/release/master/fix_qos_invalid_dscp.md b/doc/release/master/fix_qos_invalid_dscp.md new file mode 100644 index 00000000000..501714ceeec --- /dev/null +++ b/doc/release/master/fix_qos_invalid_dscp.md @@ -0,0 +1,10 @@ +fix_qos_invalid_dscp {#master} +-------------------- + +### Libraries + +#### `os` + +##### `Port` + +* Passing an invalid string when setting the QoS by DSCP no longer sets it to 0. diff --git a/src/libYARP_os/src/yarp/os/impl/PortCore.cpp b/src/libYARP_os/src/yarp/os/impl/PortCore.cpp index 9a66332aa11..044aab2ad33 100644 --- a/src/libYARP_os/src/yarp/os/impl/PortCore.cpp +++ b/src/libYARP_os/src/yarp/os/impl/PortCore.cpp @@ -2394,10 +2394,12 @@ bool PortCore::adminBlock(ConnectionReader& reader, if (portName == key) { Bottle* qos_prop = qos.find("qos").asList(); if (qos_prop != nullptr) { + int tos = -1; if (qos_prop->check("priority")) { - NetInt32 priority = qos_prop->find("priority").asVocab(); - // set the packet DSCP value based on some predefined priority levels + // set the packet TOS value on the socket based on some predefined + // priority levels. // the expected levels are: LOW, NORM, HIGH, CRIT + NetInt32 priority = qos_prop->find("priority").asVocab(); int dscp; switch (priority) { case yarp::os::createVocab('L', 'O', 'W'): @@ -2416,22 +2418,31 @@ bool PortCore::adminBlock(ConnectionReader& reader, dscp = -1; } if (dscp >= 0) { - bOk = setTypeOfService(unit, dscp << 2); + tos = (dscp << 2); } } else if (qos_prop->check("dscp")) { + // Set the packet TOS value on the socket based on the DSCP level QosStyle::PacketPriorityDSCP dscp_class = QosStyle::getDSCPByVocab(qos_prop->find("dscp").asVocab()); int dscp = -1; if (dscp_class == QosStyle::DSCP_Invalid) { - dscp = qos_prop->find("dscp").asInt32(); + auto dscp_val = qos_prop->find("dscp"); + if (dscp_val.isInt32()) { + dscp = dscp_val.asInt32(); + } } else { dscp = static_cast(dscp_class); } if ((dscp >= 0) && (dscp < 64)) { - bOk = setTypeOfService(unit, dscp << 2); + tos = (dscp << 2); } } else if (qos_prop->check("tos")) { - int tos = qos_prop->find("tos").asInt32(); - // set the TOS value (backward compatibility) + // Set the TOS value directly + auto tos_val = qos_prop->find("tos"); + if (tos_val.isInt32()) { + tos = tos_val.asInt32(); + } + } + if (tos >= 0) { bOk = setTypeOfService(unit, tos); } } else { @@ -2740,12 +2751,19 @@ bool PortCore::setTypeOfService(PortCoreUnit* unit, int tos) return false; } + yCDebug(PORTCORE, "Trying to set TOS = %d", tos); + if (unit->isOutput()) { auto* outUnit = dynamic_cast(unit); if (outUnit != nullptr) { OutputProtocol* op = outUnit->getOutPutProtocol(); if (op != nullptr) { - return op->getOutputStream().setTypeOfService(tos); + yCDebug(PORTCORE, "Trying to set TOS = %d on output unit", tos); + bool ok = op->getOutputStream().setTypeOfService(tos); + if (!ok) { + yCWarning(PORTCORE, "Setting TOS on output unit failed"); + } + return ok; } } } @@ -2760,7 +2778,12 @@ bool PortCore::setTypeOfService(PortCoreUnit* unit, int tos) if (inUnit != nullptr) { InputProtocol* ip = inUnit->getInPutProtocol(); if ((ip != nullptr) && ip->getOutput().isOk()) { - return ip->getOutput().getOutputStream().setTypeOfService(tos); + yCDebug(PORTCORE, "Trying to set TOS = %d on input unit", tos); + bool ok = ip->getOutput().getOutputStream().setTypeOfService(tos); + if (!ok) { + yCWarning(PORTCORE, "Setting TOS on input unit failed"); + } + return ok; } } } diff --git a/src/libYARP_os/src/yarp/os/impl/SocketTwoWayStream.cpp b/src/libYARP_os/src/yarp/os/impl/SocketTwoWayStream.cpp index 7cb4bd573d9..84a4c08b068 100644 --- a/src/libYARP_os/src/yarp/os/impl/SocketTwoWayStream.cpp +++ b/src/libYARP_os/src/yarp/os/impl/SocketTwoWayStream.cpp @@ -143,6 +143,7 @@ void SocketTwoWayStream::updateAddresses() bool SocketTwoWayStream::setTypeOfService(int tos) { + yCDebug(SOCKETTWOWAYSTREAM, "Setting tos = %d", tos); return (stream.set_option(IPPROTO_IP, IP_TOS, (int*)&tos, (int)sizeof(tos)) == 0); }