[ovs-dev] [PATCH v2] datapath-windows: Support attribute OVS_KEY_ATTR_TCP_FLAGS
Sorin Vinturis
svinturis at cloudbasesolutions.com
Thu Oct 22 10:36:36 UTC 2015
Hi Guru,
Alin posted the patch and I addressed the review comments, since he is in vacation.
-Sorin
-----Original Message-----
From: Gurucharan Shetty [mailto:shettyg at nicira.com]
Sent: Thursday, 22 October, 2015 02:30
To: Sorin Vinturis
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Support attribute OVS_KEY_ATTR_TCP_FLAGS
On Thu, Oct 15, 2015 at 4:58 AM, Sorin Vinturis <svinturis at cloudbasesolutions.com> wrote:
> This patch adds OVS_KEY_ATTR_TCP_FLAGS to our flow mechanism.
>
> Also clean unecesarry parts of code.
>
> Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
There are 2 Signed-off-by's. Can you please explain? If you both authored the patch, then Alin needs be a Co-authered-by instead of signed-off-by
>
> ---
> This patch is intended for branch-2.4 as well.
> ---
> datapath-windows/ovsext/DpInternal.h | 7 +++---
> datapath-windows/ovsext/Flow.c | 40 ++++++++++++++++++++++++++--------
> datapath-windows/ovsext/Flow.h | 2 +-
> datapath-windows/ovsext/PacketParser.c | 1 +
> datapath-windows/ovsext/PacketParser.h | 3 ---
> 5 files changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/datapath-windows/ovsext/DpInternal.h
> b/datapath-windows/ovsext/DpInternal.h
> index 8de48a2..0405b8e 100644
> --- a/datapath-windows/ovsext/DpInternal.h
> +++ b/datapath-windows/ovsext/DpInternal.h
> @@ -65,6 +65,8 @@ typedef struct _OVS_VPORT_EXT_INFO { typedef struct
> L4Key {
> ovs_be16 tpSrc; /* TCP/UDP/SCTP source port. */
> ovs_be16 tpDst; /* TCP/UDP/SCTP destination port. */
> + ovs_be16 flags; /* TCP flags */
> + uint8_t pad[2];
> } L4Key;
>
> typedef struct Ipkey {
> @@ -75,7 +77,7 @@ typedef struct Ipkey {
> uint8_t nwTtl; /* IP TTL/Hop Limit. */
> uint8_t nwFrag; /* FLOW_FRAG_* flags. */
> L4Key l4;
> -} IpKey; /* Size of 16 byte. */
> +} IpKey; /* Size of 20 byte. */
>
> typedef struct ArpKey {
> ovs_be32 nwSrc; /* IPv4 source address. */
> @@ -95,7 +97,6 @@ typedef struct Ipv6Key {
> uint8_t nwTtl; /* IP TTL/Hop Limit. */
> uint8_t nwFrag; /* FLOW_FRAG_* flags. */
> L4Key l4;
> - uint32_t pad;
> } Ipv6Key; /* Size of 48 byte. */
>
> typedef struct Icmp6Key {
> @@ -110,7 +111,7 @@ typedef struct Icmp6Key {
> uint8_t arpSha[6]; /* ARP/ND source hardware address. */
> uint8_t arpTha[6]; /* ARP/ND target hardware address. */
> struct in6_addr ndTarget; /* IPv6 neighbor discovery (ND) target. */
> -} Icmp6Key; /* Size of 72 byte. */
> +} Icmp6Key; /* Size of 76 byte. */
>
> typedef struct L2Key {
> uint32_t inPort; /* Port number of input port. */
> diff --git a/datapath-windows/ovsext/Flow.c
> b/datapath-windows/ovsext/Flow.c index b629c93..ca6a353 100644
> --- a/datapath-windows/ovsext/Flow.c
> +++ b/datapath-windows/ovsext/Flow.c
> @@ -980,6 +980,7 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey *ipv4FlowPutKey)
> switch (ipv4Key.ipv4_proto) {
> case IPPROTO_TCP: {
> struct ovs_key_tcp tcpKey;
> + UINT16 tcpFlags = 0;
> tcpKey.tcp_src = ipv4FlowPutKey->l4.tpSrc;
> tcpKey.tcp_dst = ipv4FlowPutKey->l4.tpDst;
> if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP, @@
> -988,6 +989,13 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey *ipv4FlowPutKey)
> rc = STATUS_UNSUCCESSFUL;
> goto done;
> }
> + tcpFlags = ipv4FlowPutKey->l4.flags;
> + if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
> + (PCHAR)(&tcpFlags),
> + sizeof(tcpFlags))) {
> + rc = STATUS_UNSUCCESSFUL;
> + goto done;
> + }
> break;
> }
>
> @@ -1074,6 +1082,7 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key *ipv6FlowPutKey,
> switch (ipv6Key.ipv6_proto) {
> case IPPROTO_TCP: {
> struct ovs_key_tcp tcpKey;
> + UINT16 tcpFlags = 0;
> tcpKey.tcp_src = ipv6FlowPutKey->l4.tpSrc;
> tcpKey.tcp_dst = ipv6FlowPutKey->l4.tpDst;
> if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP, @@
> -1082,6 +1091,13 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key *ipv6FlowPutKey,
> rc = STATUS_UNSUCCESSFUL;
> goto done;
> }
> + tcpFlags = ipv6FlowPutKey->l4.flags;
> + if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
> + (PCHAR)(&tcpFlags),
> + sizeof(tcpFlags))) {
> + rc = STATUS_UNSUCCESSFUL;
> + goto done;
> + }
> break;
> }
>
> @@ -1357,6 +1373,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
> ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
> }
>
> + if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
> + const UINT16 *flags;
> + flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
> + ipv4FlowPutKey->l4.flags = *flags;
> + }
> +
> if (keyAttrs[OVS_KEY_ATTR_UDP]) {
> const struct ovs_key_udp *udpKey;
> udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
> @@ -1401,6 +1423,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
> ipv6FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
> }
>
> + if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
> + const UINT16 *flags;
> + flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
> + ipv6FlowPutKey->l4.flags = *flags;
> + }
> +
> if (keyAttrs[OVS_KEY_ATTR_UDP]) {
> const struct ovs_key_udp *udpKey;
> udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
> @@ -1443,8 +1471,6 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
>
> destKey->l2.keyLen += OVS_IPV6_KEY_SIZE;
> }
> -
> - ipv6FlowPutKey->pad = 0;
> }
> break;
> }
> @@ -1465,9 +1491,6 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
> /* Kernel datapath assumes 'arpFlowPutKey->nwProto' to be in host
> * order. */
> arpFlowPutKey->nwProto = (UINT8)ntohs((arpKey->arp_op));
> - arpFlowPutKey->pad[0] = 0;
> - arpFlowPutKey->pad[1] = 0;
> - arpFlowPutKey->pad[2] = 0;
> destKey->l2.keyLen += OVS_ARP_KEY_SIZE;
> break;
> }
> @@ -1635,7 +1658,7 @@ OvsFlowUsed(OvsFlow *flow,
> flow->used = tickCount.QuadPart * ovsTimeIncrementPerTick;
> flow->packetCount++;
> flow->byteCount += OvsPacketLenNBL(packet);
> - flow->tcpFlags |= OvsGetTcpFlags(packet, &flow->key, layers);
> + flow->tcpFlags |= OvsGetTcpFlags(packet, layers);
> }
>
>
> @@ -1796,6 +1819,7 @@ OvsExtractFlow(const NET_BUFFER_LIST *packet,
> ipKey->nwTtl = nh->ttl;
> ipKey->l4.tpSrc = 0;
> ipKey->l4.tpDst = 0;
> + ipKey->l4.flags = 0;
>
> if (!(nh->frag_off & htons(IP_OFFSET))) {
> if (ipKey->nwProto == SOCKET_IPPROTO_TCP) { @@
> -1829,7 +1853,7 @@ OvsExtractFlow(const NET_BUFFER_LIST *packet,
> layers->isIPv6 = 1;
> flow->ipv6Key.l4.tpSrc = 0;
> flow->ipv6Key.l4.tpDst = 0;
> - flow->ipv6Key.pad = 0;
> + flow->ipv6Key.l4.flags = 0;
>
> if (flow->ipv6Key.nwProto == SOCKET_IPPROTO_TCP) {
> OvsParseTcp(packet, &(flow->ipv6Key.l4), layers); @@
> -1910,9 +1934,7 @@ AddFlow(OVS_DATAPATH *datapath, OvsFlow *flow)
> */
> KeMemoryBarrier();
>
> - //KeAcquireSpinLock(&FilterDeviceExtension->NblQueueLock, &oldIrql);
> InsertTailList(head, &flow->ListEntry);
> - //KeReleaseSpinLock(&FilterDeviceExtension->NblQueueLock, oldIrql);
>
> datapath->nFlows++;
>
> diff --git a/datapath-windows/ovsext/Flow.h
> b/datapath-windows/ovsext/Flow.h index e817bcf..5d74ca8 100644
> --- a/datapath-windows/ovsext/Flow.h
> +++ b/datapath-windows/ovsext/Flow.h
> @@ -28,7 +28,7 @@ typedef struct _OvsFlow {
> OvsFlowKey key;
> UINT64 hash;
> UINT32 actionsLen;
> - UINT8 tcpFlags;
> + UINT8 tcpFlags;
> UINT64 used;
> UINT64 packetCount;
> UINT64 byteCount;
> diff --git a/datapath-windows/ovsext/PacketParser.c
> b/datapath-windows/ovsext/PacketParser.c
> index e01be17..219ddac 100644
> --- a/datapath-windows/ovsext/PacketParser.c
> +++ b/datapath-windows/ovsext/PacketParser.c
> @@ -186,6 +186,7 @@ OvsParseTcp(const NET_BUFFER_LIST *packet,
> if (tcp) {
> flow->tpSrc = tcp->source;
> flow->tpDst = tcp->dest;
> + flow->flags = OvsGetTcpFlags(packet, layers);
> layers->isTcp = 1;
> layers->l7Offset = layers->l4Offset + 4 * tcp->doff;
> }
> diff --git a/datapath-windows/ovsext/PacketParser.h
> b/datapath-windows/ovsext/PacketParser.h
> index 55d110f..765819a 100644
> --- a/datapath-windows/ovsext/PacketParser.h
> +++ b/datapath-windows/ovsext/PacketParser.h
> @@ -75,11 +75,8 @@ OvsGetTcpCtl(const NET_BUFFER_LIST *packet, // IN
>
> static UINT8
> OvsGetTcpFlags(const NET_BUFFER_LIST *packet, // IN
> - const OvsFlowKey *key, // IN
> const POVS_PACKET_HDR_INFO layers) // IN {
> - UNREFERENCED_PARAMETER(key); // should be removed later
> -
> if (layers->isTcp) {
> return TCP_FLAGS(OvsGetTcpCtl(packet, layers));
> } else {
> --
> 1.9.0.msysgit.0
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list