Re: [ovs-dev] [PATCH v4] Support for match & set ICMPv6 reserved and options type fields
> > Currently OVS supports all ARP protocol fields as OXM match fields to > implement the relevant ARP procedures for IPv4. This includes support > for matching copying and setting ARP fields. In IPv6 ARP has been > replaced by ICMPv6 neighbor discovery (ND) procedures, neighbor > advertisement and neighbor solicitation. > > The support for ICMPv6 fields in OVS is not complete for the use cases > equivalent to ARP in IPv4. OVS lacks support for matching, copying and > setting the “ND option type” and “ND reserved” fields. Without these user > cannot implement all ICMPv6 ND procedures for IPv6 support. > > This commit adds additional OXM fields to OVS for ICMPv6 “ND option type“ > and ICMPv6 “ND reserved” using the OXM extension mechanism. This allows > support for parsing these fields from an ICMPv6 packet header and extending > the OpenFlow protocol with specifications for these new OXM fields for > matching, copying and setting. > Hi Ben, Does the changes in V4 look ok? Warm Regards, Vishal Ajmera ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v3] conntrack: Fix possibly uninitialized memory.
There are a few cases where padding may be undefined according to
the C standard. Practically, it seems implementations don't have issue,
but it is better to be safe. The code paths modified are not hot ones.
Found by inspection.
Signed-off-by: Darrell Ball
---
v3: Removed an unnecessary change and limited scope of 2 others.
v2: Found another one; will double check for others later.
lib/conntrack.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index e1f4041..4c8d71b 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -748,6 +748,7 @@ conn_lookup(struct conntrack *ct, const struct conn_key
*key, long long now)
{
struct conn_lookup_ctx ctx;
ctx.conn = NULL;
+memset(&ctx.key, 0, sizeof ctx.key);
ctx.key = *key;
ctx.hash = conn_key_hash(key, ct->hash_basis);
unsigned bucket = hash_to_bucket(ctx.hash);
@@ -896,6 +897,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
if (nat_action_info) {
nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info);
+memset(conn_for_un_nat_copy, 0, sizeof *conn_for_un_nat_copy);
if (alg_exp) {
if (alg_exp->nat_rpl_dst) {
@@ -934,8 +936,6 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
ct_rwlock_unlock(&ct->resources_lock);
}
conn_for_un_nat_copy->conn_type = CT_CONN_TYPE_UN_NAT;
-conn_for_un_nat_copy->nat_info = NULL;
-conn_for_un_nat_copy->alg = NULL;
nat_packet(pkt, nc, ctx->icmp_related);
}
hmap_insert(&ct->buckets[bucket].connections, &nc->node, ctx->hash);
@@ -1262,6 +1262,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
ct->hash_basis,
alg_src_ip_wc(ct_alg_ctl));
if (alg_exp) {
+memset(&alg_exp_entry, 0, sizeof alg_exp_entry);
alg_exp_entry = *alg_exp;
alg_exp = &alg_exp_entry;
}
@@ -2614,7 +2615,9 @@ static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key,
uint32_t basis, bool src_ip_wc)
{
-struct conn_key check_key = *key;
+struct conn_key check_key;
+memset(&check_key, 0, sizeof check_key);
+check_key = *key;
check_key.src.port = ALG_WC_SRC_PORT;
if (src_ip_wc) {
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next V2 1/1] openvswitch: Declare ovs key structures using macros
On Sun, Feb 3, 2019 at 1:12 AM Eli Britstein wrote:
>
> Declare ovs key structures using macros as a pre-step towards to
> enable retrieving fields information, as a work done in proposed
> commit in the OVS tree https://patchwork.ozlabs.org/patch/1023406/
> ("odp-util: Do not rewrite fields with the same values as matched"),
> with no functional change.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Roi Dayan
> ---
> include/uapi/linux/openvswitch.h | 102
++-
> 1 file changed, 69 insertions(+), 33 deletions(-)
>
Thanks.
Acked-by: Pravin B Shelar
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] lib/tc: Support optional tunnel id
On Fri, Feb 1, 2019 at 4:05 PM Simon Horman wrote:
>
> Thanks Roi,
>
> On Thu, 31 Jan 2019 at 15:52, Roi Dayan wrote:
>
> >
> >
> > On 31/01/2019 15:32, Roi Dayan wrote:
> > >
> > > On 31/01/2019 11:58, Simon Horman wrote:
> > >> On Mon, Jan 21, 2019 at 05:32:37PM +0200, Adi Nissim wrote:
> > >>> Currently the TC tunnel_key action is always
> > >>> initialized with the given tunnel id value. However,
> > >>> some tunneling protocols define the tunnel id as an optional field.
> > >>>
> > >>> This patch initializes the id field of tunnel_key:set and
> > tunnel_key:unset
> > >>> only if a value is provided.
> > >>>
> > >>> In the case that a tunnel key value is not provided by the user
> > >>> the key flag will not be set.
> > >>>
> > >>> Signed-off-by: Adi Nissim
> > >>> Acked-by: Paul Blakey
> > >>> ---
> > >>> v1->v2: check if mask.tunnel.id == OVS_BE64_MAX
> > >>> so we won't do match in the case of a partial mask.
> > >> I am still a bit concerned about the partial mask case.
> > >> It looks like the code will now silently not offload such matches.
> > >>
> > >> I think that a partial mask should either be offloaded or
> > >> offload of the entire flow should be rejected.
> > > thanks. you are right. I didn't notice it. partial masks should be
> > rejected
> > > to fallback to ovs dp instead of ignoring the mask.
> > >
> >
> >
> > Hi Simon,
> >
> > I did some checks and think the correct fix is to offload exact match.
> > if key is partial we can ignore the mask and offload exact match and
> > it will be correct as we do more strict matching.
> >
> > it is also documented that the kernel datapath is doing the same
> > (from datapath.rst)
> >
> > "The kernel can ignore the mask attribute, installing an exact
> > match flow"
> >
> > So I think the first patch V0 is actually correct as we
> > check the tunnel key flag exists and offload exact match if
> > there was any mask or offload without a key if the mask is 0
> > or no key.
> >
> > in netdev-tc-offloads.c
> >
> > +flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
> > + tnl_mask->tun_id : 0;
> >
>
> I think this is fine so long as tun_id is all-ones. Is that always the case?
> Should the code check that it is the case? Am I missing the point?
>
it looks like tun_id mask is always set to all-ones.
but even if it won't be in the future, we shouldn't really care here.
tc adds exact match on the tun_id and ignores the tun_id mask.
this is considered ok as the matching is more strict.
if new match is needed with different tun_id then ovs will try to add
another rule for it.
so with tc we could have multiple rules vs 1 rule that support mask.
>
> >
> >
> > in tc.c
> >
> > -nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> > +if (id_mask) {
> > +nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> > +}
> >
> >
> > let me know what you think.
> >
> > Thanks,
> > Roi
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net-next V2 1/1] openvswitch: Declare ovs key structures using macros
Declare ovs key structures using macros as a pre-step towards to
enable retrieving fields information, as a work done in proposed
commit in the OVS tree https://patchwork.ozlabs.org/patch/1023406/
("odp-util: Do not rewrite fields with the same values as matched"),
with no functional change.
Signed-off-by: Eli Britstein
Reviewed-by: Roi Dayan
---
include/uapi/linux/openvswitch.h | 102 ++-
1 file changed, 69 insertions(+), 33 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dbe0cbe4f1b7..dc8246f871fd 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -387,73 +387,109 @@ enum ovs_frag_type {
#define OVS_FRAG_TYPE_MAX (__OVS_FRAG_TYPE_MAX - 1)
+#define OVS_KEY_FIELD_ARR(type, name, elements) type name[elements];
+#define OVS_KEY_FIELD(type, name) type name;
+
+#define OVS_KEY_ETHERNET_FIELDS \
+OVS_KEY_FIELD_ARR(__u8, eth_src, ETH_ALEN) \
+OVS_KEY_FIELD_ARR(__u8, eth_dst, ETH_ALEN)
+
struct ovs_key_ethernet {
- __u8 eth_src[ETH_ALEN];
- __u8 eth_dst[ETH_ALEN];
+OVS_KEY_ETHERNET_FIELDS
};
struct ovs_key_mpls {
__be32 mpls_lse;
};
+#define OVS_KEY_IPV4_FIELDS \
+OVS_KEY_FIELD(__be32, ipv4_src) \
+OVS_KEY_FIELD(__be32, ipv4_dst) \
+OVS_KEY_FIELD(__u8, ipv4_proto) \
+OVS_KEY_FIELD(__u8, ipv4_tos) \
+OVS_KEY_FIELD(__u8, ipv4_ttl) \
+OVS_KEY_FIELD(__u8, ipv4_frag /* One of OVS_FRAG_TYPE_*. */)
+
struct ovs_key_ipv4 {
- __be32 ipv4_src;
- __be32 ipv4_dst;
- __u8 ipv4_proto;
- __u8 ipv4_tos;
- __u8 ipv4_ttl;
- __u8 ipv4_frag; /* One of OVS_FRAG_TYPE_*. */
+OVS_KEY_IPV4_FIELDS
};
+#define OVS_KEY_IPV6_FIELDS \
+OVS_KEY_FIELD_ARR(__be32, ipv6_src, 4) \
+OVS_KEY_FIELD_ARR(__be32, ipv6_dst, 4) \
+OVS_KEY_FIELD(__be32, ipv6_label /* 20-bits in least-significant bits. */)
\
+OVS_KEY_FIELD(__u8, ipv6_proto) \
+OVS_KEY_FIELD(__u8, ipv6_tclass) \
+OVS_KEY_FIELD(__u8, ipv6_hlimit) \
+OVS_KEY_FIELD(__u8, ipv6_frag /* One of OVS_FRAG_TYPE_*. */)
+
struct ovs_key_ipv6 {
- __be32 ipv6_src[4];
- __be32 ipv6_dst[4];
- __be32 ipv6_label; /* 20-bits in least-significant bits. */
- __u8 ipv6_proto;
- __u8 ipv6_tclass;
- __u8 ipv6_hlimit;
- __u8 ipv6_frag; /* One of OVS_FRAG_TYPE_*. */
+OVS_KEY_IPV6_FIELDS
};
+#define OVS_KEY_TCP_FIELDS \
+OVS_KEY_FIELD(__be16, tcp_src) \
+OVS_KEY_FIELD(__be16, tcp_dst)
+
struct ovs_key_tcp {
- __be16 tcp_src;
- __be16 tcp_dst;
+OVS_KEY_TCP_FIELDS
};
+#define OVS_KEY_UDP_FIELDS \
+OVS_KEY_FIELD(__be16, udp_src) \
+OVS_KEY_FIELD(__be16, udp_dst)
+
struct ovs_key_udp {
- __be16 udp_src;
- __be16 udp_dst;
+OVS_KEY_UDP_FIELDS
};
+#define OVS_KEY_SCTP_FIELDS \
+OVS_KEY_FIELD(__be16, sctp_src) \
+OVS_KEY_FIELD(__be16, sctp_dst)
+
struct ovs_key_sctp {
- __be16 sctp_src;
- __be16 sctp_dst;
+OVS_KEY_SCTP_FIELDS
};
+#define OVS_KEY_ICMP_FIELDS \
+OVS_KEY_FIELD(__u8, icmp_type) \
+OVS_KEY_FIELD(__u8, icmp_code)
+
struct ovs_key_icmp {
- __u8 icmp_type;
- __u8 icmp_code;
+OVS_KEY_ICMP_FIELDS
};
+#define OVS_KEY_ICMPV6_FIELDS \
+OVS_KEY_FIELD(__u8, icmpv6_type) \
+OVS_KEY_FIELD(__u8, icmpv6_code)
+
struct ovs_key_icmpv6 {
- __u8 icmpv6_type;
- __u8 icmpv6_code;
+OVS_KEY_ICMPV6_FIELDS
};
+#define OVS_KEY_ARP_FIELDS \
+OVS_KEY_FIELD(__be32, arp_sip) \
+OVS_KEY_FIELD(__be32, arp_tip) \
+OVS_KEY_FIELD(__be16, arp_op) \
+OVS_KEY_FIELD_ARR(__u8, arp_sha, ETH_ALEN) \
+OVS_KEY_FIELD_ARR(__u8, arp_tha, ETH_ALEN)
+
struct ovs_key_arp {
- __be32 arp_sip;
- __be32 arp_tip;
- __be16 arp_op;
- __u8 arp_sha[ETH_ALEN];
- __u8 arp_tha[ETH_ALEN];
+OVS_KEY_ARP_FIELDS
};
+#define OVS_KEY_ND_FIELDS \
+OVS_KEY_FIELD_ARR(__be32, nd_target, 4) \
+OVS_KEY_FIELD_ARR(__u8, nd_sll, ETH_ALEN) \
+OVS_KEY_FIELD_ARR(__u8, nd_tll, ETH_ALEN)
+
struct ovs_key_nd {
- __be32 nd_target[4];
- __u8nd_sll[ETH_ALEN];
- __u8nd_tll[ETH_ALEN];
+OVS_KEY_ND_FIELDS
};
+#undef OVS_KEY_FIELD_ARR
+#undef OVS_KEY_FIELD
+
#define OVS_CT_LABELS_LEN_32 4
#define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32))
struct ovs_key_ct_labels {
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
