[ovs-dev] [PATCH v2 2/2] datapath-windows: Accept MPLS feature probe.
Nithin Raju
nithin at vmware.com
Fri Jan 29 19:35:08 UTC 2016
The current code is Flow.c is not great in terms of communicating what is
supported and not supported. Ideally, we should be doing something like
this:
1) Parse Flow attributes, and flag the flow if mandatory attributes such
as KEY are missing.
2) Handle unsupported such as UFID by returning EINVAL explicitly. Looks
like we only support KEY, ACTIONS, STATS, everything else should return
EINVAL.
3) Parse each nested attribute, and give it the same treatment as #1 and
#2 ie. use netlink parsing to catch mandatory attributes, and explicit
flagging for unsupported attributes.
4) If PROBE is sent down, have the same treatment as #1, #2, #3. Only
thing that changes is avoiding logging.
I am ok with the implementation you have in the meantime.
Acked-by: Nithin Raju <nithin at vmware.com>
Thanks,
-- Nithin
-----Original Message-----
From: dev <dev-bounces at openvswitch.org> on behalf of Sorin Vinturis
<svinturis at cloudbasesolutions.com>
Date: Tuesday, January 19, 2016 at 12:04 PM
To: "dev at openvswitch.org" <dev at openvswitch.org>
Subject: [ovs-dev] [PATCH v2 2/2] datapath-windows: Accept MPLS
feature probe.
>Currently all feature probe messages sent from userspace are
>suppressed by the OVS extension.
>
>This patch changes the current behaviour to allow feature probe
>for MPLS.
>
>Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
>---
>v2: Added NL buffer parsing in feature probe function.
>---
> datapath-windows/ovsext/Flow.c | 72
>+++++++++++++++++++++++---
> datapath-windows/ovsext/Mpls.h | 24 +++++++++
> datapath-windows/ovsext/Netlink/NetlinkError.h | 3 ++
> 3 files changed, 92 insertions(+), 7 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Flow.c
>b/datapath-windows/ovsext/Flow.c
>index 678c841..ea6baa0 100644
>--- a/datapath-windows/ovsext/Flow.c
>+++ b/datapath-windows/ovsext/Flow.c
>@@ -86,6 +86,8 @@ static NTSTATUS _MapFlowMplsKeyToNlKey(PNL_BUFFER nlBuf,
> static NTSTATUS OvsDoDumpFlows(OvsFlowDumpInput *dumpInput,
> OvsFlowDumpOutput *dumpOutput,
> UINT32 *replyLen);
>+static NTSTATUS OvsProbeSupportedFeature(POVS_MESSAGE msgIn,
>+ PNL_ATTR keyAttr);
>
>
> #define OVS_FLOW_TABLE_SIZE 2048
>@@ -313,13 +315,17 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
> }
>
> if (flowAttrs[OVS_FLOW_ATTR_PROBE]) {
>- OVS_LOG_ERROR("Attribute OVS_FLOW_ATTR_PROBE not supported");
>- goto done;
>+ rc = OvsProbeSupportedFeature(msgIn,
>flowAttrs[OVS_FLOW_ATTR_KEY]);
>+ if (rc != STATUS_SUCCESS) {
>+ nlError = NlMapStatusToNlErr(rc);
>+ goto done;
>+ }
> }
>
> if ((rc = _MapNlToFlowPut(msgIn, flowAttrs[OVS_FLOW_ATTR_KEY],
>- flowAttrs[OVS_FLOW_ATTR_ACTIONS], flowAttrs[OVS_FLOW_ATTR_CLEAR],
>- &mappedFlow))
>+ flowAttrs[OVS_FLOW_ATTR_ACTIONS],
>+ flowAttrs[OVS_FLOW_ATTR_CLEAR],
>+ &mappedFlow))
> != STATUS_SUCCESS) {
> OVS_LOG_ERROR("Conversion to OvsFlowPut failed");
> goto done;
>@@ -1254,7 +1260,7 @@ _MapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR
>keyAttr,
> keyAttrs, ARRAY_SIZE(keyAttrs)))
> != TRUE) {
> OVS_LOG_ERROR("Key Attr Parsing failed for msg: %p",
>- nlMsgHdr);
>+ nlMsgHdr);
> rc = STATUS_INVALID_PARAMETER;
> goto done;
> }
>@@ -1272,7 +1278,7 @@ _MapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR
>keyAttr,
> tunnelAttrs, ARRAY_SIZE(tunnelAttrs)))
> != TRUE) {
> OVS_LOG_ERROR("Tunnel key Attr Parsing failed for msg: %p",
>- nlMsgHdr);
>+ nlMsgHdr);
> rc = STATUS_INVALID_PARAMETER;
> goto done;
> }
>@@ -1290,7 +1296,7 @@ _MapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR
>keyAttr,
> mappedFlow->dpNo = ovsHdr->dp_ifindex;
>
> _MapNlToFlowPutFlags(genlMsgHdr, flowAttrClear,
>- mappedFlow);
>+ mappedFlow);
>
> done:
> return rc;
>@@ -2521,4 +2527,56 @@ OvsTunKeyAttrSize(void)
> + NlAttrTotalSize(2); /* OVS_TUNNEL_KEY_ATTR_TP_DST */
> }
>
>+/*
>+
>*-------------------------------------------------------------------------
>---
>+ * OvsProbeSupportedFeature --
>+ * Verifies if the probed feature is supported.
>+ *
>+ * Results:
>+ * STATUS_SUCCESS if the probed feature is supported.
>+
>*-------------------------------------------------------------------------
>---
>+ */
>+static NTSTATUS
>+OvsProbeSupportedFeature(POVS_MESSAGE msgIn,
>+ PNL_ATTR keyAttr)
>+{
>+ NTSTATUS status = STATUS_SUCCESS;
>+ PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg);
>+
>+ UINT32 keyAttrOffset = (UINT32)((PCHAR)keyAttr - (PCHAR)nlMsgHdr);
>+ PNL_ATTR keyAttrs[__OVS_KEY_ATTR_MAX] = { NULL };
>+
>+ /* Get flow keys attributes */
>+ if ((NlAttrParseNested(nlMsgHdr, keyAttrOffset, NlAttrLen(keyAttr),
>+ nlFlowKeyPolicy, ARRAY_SIZE(nlFlowKeyPolicy),
>+ keyAttrs, ARRAY_SIZE(keyAttrs)))
>+ != TRUE) {
>+ OVS_LOG_ERROR("Key Attr Parsing failed for msg: %p",
>+ nlMsgHdr);
>+ status = STATUS_INVALID_PARAMETER;
>+ goto done;
>+ }
>+
>+ if (keyAttrs[OVS_KEY_ATTR_MPLS] &&
>+ keyAttrs[OVS_KEY_ATTR_ETHERTYPE]) {
>+ ovs_be16 ethType =
>NlAttrGetU16(keyAttrs[OVS_KEY_ATTR_ETHERTYPE]);
>+
>+ if (OvsEthertypeIsMpls(ethType)) {
>+ if (!OvsCountMplsLabels(keyAttrs[OVS_KEY_ATTR_MPLS])) {
>+ OVS_LOG_ERROR("Maximum supported MPLS labels exceeded.");
>+ status = STATUS_INVALID_MESSAGE;
>+ }
>+ } else {
>+ OVS_LOG_ERROR("Wrong ethertype for MPLS attribute.");
>+ status = STATUS_INVALID_PARAMETER;
>+ }
>+ } else {
>+ OVS_LOG_ERROR("Probed feature not supported.");
>+ status = STATUS_INVALID_PARAMETER;
>+ }
>+
>+done:
>+ return status;
>+}
>+
> #pragma warning( pop )
>diff --git a/datapath-windows/ovsext/Mpls.h
>b/datapath-windows/ovsext/Mpls.h
>index 6e53e46..084613f 100644
>--- a/datapath-windows/ovsext/Mpls.h
>+++ b/datapath-windows/ovsext/Mpls.h
>@@ -53,4 +53,28 @@ OvsEthertypeIsMpls(ovs_be16 ethertype)
> ethertype == htons(ETH_TYPE_MPLS_MCAST);
> }
>
>+/* Returns the number of MPLS LSEs present in 'a'
>+ *
>+ * Counts 'flow''s MPLS label stack entries (LESs) stopping at the first
>+ * entry that has the bottom of stack (BOS) bit set. If no such entry
>exists,
>+ * then zero is returned, meaning that the maximum number of supported
>+ * MPLS LSEs exceeded.
>+ */
>+__inline UINT32
>+OvsCountMplsLabels(PNL_ATTR a)
>+{
>+ const MPLSHdr *mpls = NlAttrGet(a);
>+ UINT32 count = 0;
>+ BOOLEAN bos = FALSE;
>+
>+ for (count = 0; count < FLOW_MAX_MPLS_LABELS; count++) {
>+ if ((mpls + count)->lse & htonl(MPLS_BOS_MASK)) {
>+ bos = TRUE;
>+ break;
>+ }
>+ }
>+
>+ return bos ? ++count : 0;
>+}
>+
> #endif /* __MPLS_H_ */
>diff --git a/datapath-windows/ovsext/Netlink/NetlinkError.h
>b/datapath-windows/ovsext/Netlink/NetlinkError.h
>index eefa89e..36115c8 100644
>--- a/datapath-windows/ovsext/Netlink/NetlinkError.h
>+++ b/datapath-windows/ovsext/Netlink/NetlinkError.h
>@@ -229,6 +229,9 @@ NlMapStatusToNlErr(NTSTATUS status)
> case STATUS_OBJECT_NAME_EXISTS:
> ret = NL_ERROR_EXIST;
> break;
>+ case STATUS_INVALID_MESSAGE:
>+ ret = NL_ERROR_BADMSG;
>+ break;
> default:
> ret = NL_ERROR_OTHER;
> break;
>--
>1.9.0.msysgit.0
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=wtOwkVOFiPbBiRkTMf5pWgAbM1-Ga0
>yH3__zd96_WqY&s=5r7NCPrFaeT9aGOeNwvA1uMM8laMZWyMJYQiP1DnWgA&e=
More information about the dev
mailing list