Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1235618 > unrolled thread
| Started by | Joe Stringer <joestringer@nicira.com> |
|---|---|
| First post | 2015-09-30 00:50 +0200 |
| Last post | 2015-10-01 04:40 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH net 5/7] openvswitch: Reject ct_state unsupported bits Joe Stringer <joestringer@nicira.com> - 2015-09-30 00:50 +0200
Re: [PATCH net 5/7] openvswitch: Reject ct_state unsupported bits Pravin Shelar <pshelar@nicira.com> - 2015-10-01 02:40 +0200
Re: [PATCH net 5/7] openvswitch: Reject ct_state unsupported bits Joe Stringer <joestringer@nicira.com> - 2015-10-01 03:30 +0200
Re: [PATCH net 5/7] openvswitch: Reject ct_state unsupported bits Pravin Shelar <pshelar@nicira.com> - 2015-10-01 04:40 +0200
| From | Joe Stringer <joestringer@nicira.com> |
|---|---|
| Date | 2015-09-30 00:50 +0200 |
| Subject | [PATCH net 5/7] openvswitch: Reject ct_state unsupported bits |
| Message-ID | <qebSH-3fs-31@gated-at.bofh.it> |
Previously, if userspace specified ct_state bits in the flow key which
are currently undefined (and therefore unsupported), then they would be
ignored. This could cause unexpected behaviour in future if userspace is
extended to support additional bits but attempts to communicate with the
current version of the kernel. This patch rectifies the situation by
rejecting such ct_state bits.
Fixes: 7f8a436 "openvswitch: Add conntrack action"
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
net/openvswitch/conntrack.h | 12 ++++++++++++
net/openvswitch/flow_netlink.c | 6 ++++++
2 files changed, 18 insertions(+)
diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
index 43f5dd7..c658d95 100644
--- a/net/openvswitch/conntrack.h
+++ b/net/openvswitch/conntrack.h
@@ -34,6 +34,13 @@ int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
int ovs_ct_put_key(const struct sw_flow_key *key, struct sk_buff *skb);
void ovs_ct_free_action(const struct nlattr *a);
+
+static inline bool ovs_ct_state_supported(u8 state)
+{
+ return !(state & ~(OVS_CS_F_NEW | OVS_CS_F_ESTABLISHED |
+ OVS_CS_F_RELATED | OVS_CS_F_REPLY_DIR |
+ OVS_CS_F_INVALID | OVS_CS_F_TRACKED));
+}
#else
#include <linux/errno.h>
@@ -46,6 +53,11 @@ static inline bool ovs_ct_verify(struct net *net, int attr)
return false;
}
+static inline bool ovs_ct_state_supported(u8 state)
+{
+ return false;
+}
+
static inline int ovs_ct_copy_action(struct net *net, const struct nlattr *nla,
const struct sw_flow_key *key,
struct sw_flow_actions **acts, bool log)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index ea82cd5..c4917c9 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -816,6 +816,12 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
ovs_ct_verify(net, OVS_KEY_ATTR_CT_STATE)) {
u8 ct_state = nla_get_u8(a[OVS_KEY_ATTR_CT_STATE]);
+ if (!is_mask && !ovs_ct_state_supported(ct_state)) {
+ OVS_NLERR(log, "ct_state flags %02x unsupported",
+ ct_state);
+ return -EINVAL;
+ }
+
SW_FLOW_KEY_PUT(match, ct.state, ct_state, is_mask);
*attrs &= ~(1ULL << OVS_KEY_ATTR_CT_STATE);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Pravin Shelar <pshelar@nicira.com> |
|---|---|
| Date | 2015-10-01 02:40 +0200 |
| Message-ID | <qeA4G-4qI-7@gated-at.bofh.it> |
| In reply to | #1235618 |
On Tue, Sep 29, 2015 at 3:39 PM, Joe Stringer <joestringer@nicira.com> wrote:
> Previously, if userspace specified ct_state bits in the flow key which
> are currently undefined (and therefore unsupported), then they would be
> ignored. This could cause unexpected behaviour in future if userspace is
> extended to support additional bits but attempts to communicate with the
> current version of the kernel. This patch rectifies the situation by
> rejecting such ct_state bits.
>
> Fixes: 7f8a436 "openvswitch: Add conntrack action"
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> net/openvswitch/conntrack.h | 12 ++++++++++++
> net/openvswitch/flow_netlink.c | 6 ++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
> index 43f5dd7..c658d95 100644
> --- a/net/openvswitch/conntrack.h
> +++ b/net/openvswitch/conntrack.h
> @@ -34,6 +34,13 @@ int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
> void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
> int ovs_ct_put_key(const struct sw_flow_key *key, struct sk_buff *skb);
> void ovs_ct_free_action(const struct nlattr *a);
> +
> +static inline bool ovs_ct_state_supported(u8 state)
> +{
> + return !(state & ~(OVS_CS_F_NEW | OVS_CS_F_ESTABLISHED |
> + OVS_CS_F_RELATED | OVS_CS_F_REPLY_DIR |
> + OVS_CS_F_INVALID | OVS_CS_F_TRACKED));
> +}
> #else
> #include <linux/errno.h>
>
> @@ -46,6 +53,11 @@ static inline bool ovs_ct_verify(struct net *net, int attr)
> return false;
> }
>
> +static inline bool ovs_ct_state_supported(u8 state)
> +{
> + return false;
> +}
> +
> static inline int ovs_ct_copy_action(struct net *net, const struct nlattr *nla,
> const struct sw_flow_key *key,
> struct sw_flow_actions **acts, bool log)
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index ea82cd5..c4917c9 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -816,6 +816,12 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
> ovs_ct_verify(net, OVS_KEY_ATTR_CT_STATE)) {
> u8 ct_state = nla_get_u8(a[OVS_KEY_ATTR_CT_STATE]);
>
We also need to return error if kernel does not support the feature.
> + if (!is_mask && !ovs_ct_state_supported(ct_state)) {
> + OVS_NLERR(log, "ct_state flags %02x unsupported",
> + ct_state);
> + return -EINVAL;
> + }
> +
> SW_FLOW_KEY_PUT(match, ct.state, ct_state, is_mask);
> *attrs &= ~(1ULL << OVS_KEY_ATTR_CT_STATE);
> }
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Stringer <joestringer@nicira.com> |
|---|---|
| Date | 2015-10-01 03:30 +0200 |
| Message-ID | <qeAR3-5zy-3@gated-at.bofh.it> |
| In reply to | #1236876 |
On 30 September 2015 at 17:31, Pravin Shelar <pshelar@nicira.com> wrote:
> On Tue, Sep 29, 2015 at 3:39 PM, Joe Stringer <joestringer@nicira.com> wrote:
>> Previously, if userspace specified ct_state bits in the flow key which
>> are currently undefined (and therefore unsupported), then they would be
>> ignored. This could cause unexpected behaviour in future if userspace is
>> extended to support additional bits but attempts to communicate with the
>> current version of the kernel. This patch rectifies the situation by
>> rejecting such ct_state bits.
>>
>> Fixes: 7f8a436 "openvswitch: Add conntrack action"
>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>> ---
>> net/openvswitch/conntrack.h | 12 ++++++++++++
>> net/openvswitch/flow_netlink.c | 6 ++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
>> index 43f5dd7..c658d95 100644
>> --- a/net/openvswitch/conntrack.h
>> +++ b/net/openvswitch/conntrack.h
>> @@ -34,6 +34,13 @@ int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
>> void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
>> int ovs_ct_put_key(const struct sw_flow_key *key, struct sk_buff *skb);
>> void ovs_ct_free_action(const struct nlattr *a);
>> +
>> +static inline bool ovs_ct_state_supported(u8 state)
>> +{
>> + return !(state & ~(OVS_CS_F_NEW | OVS_CS_F_ESTABLISHED |
>> + OVS_CS_F_RELATED | OVS_CS_F_REPLY_DIR |
>> + OVS_CS_F_INVALID | OVS_CS_F_TRACKED));
>> +}
>> #else
>> #include <linux/errno.h>
>>
>> @@ -46,6 +53,11 @@ static inline bool ovs_ct_verify(struct net *net, int attr)
>> return false;
>> }
>>
>> +static inline bool ovs_ct_state_supported(u8 state)
>> +{
>> + return false;
>> +}
>> +
>> static inline int ovs_ct_copy_action(struct net *net, const struct nlattr *nla,
>> const struct sw_flow_key *key,
>> struct sw_flow_actions **acts, bool log)
>> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
>> index ea82cd5..c4917c9 100644
>> --- a/net/openvswitch/flow_netlink.c
>> +++ b/net/openvswitch/flow_netlink.c
>> @@ -816,6 +816,12 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
>> ovs_ct_verify(net, OVS_KEY_ATTR_CT_STATE)) {
>> u8 ct_state = nla_get_u8(a[OVS_KEY_ATTR_CT_STATE]);
>>
> We also need to return error if kernel does not support the feature.
Already handled by ovs_ct_verify() in conntrack.h.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Pravin Shelar <pshelar@nicira.com> |
|---|---|
| Date | 2015-10-01 04:40 +0200 |
| Message-ID | <qeBWN-7aJ-5@gated-at.bofh.it> |
| In reply to | #1236893 |
On Wed, Sep 30, 2015 at 6:20 PM, Joe Stringer <joestringer@nicira.com> wrote:
> On 30 September 2015 at 17:31, Pravin Shelar <pshelar@nicira.com> wrote:
>> On Tue, Sep 29, 2015 at 3:39 PM, Joe Stringer <joestringer@nicira.com> wrote:
>>> Previously, if userspace specified ct_state bits in the flow key which
>>> are currently undefined (and therefore unsupported), then they would be
>>> ignored. This could cause unexpected behaviour in future if userspace is
>>> extended to support additional bits but attempts to communicate with the
>>> current version of the kernel. This patch rectifies the situation by
>>> rejecting such ct_state bits.
>>>
>>> Fixes: 7f8a436 "openvswitch: Add conntrack action"
>>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>>> ---
>>> net/openvswitch/conntrack.h | 12 ++++++++++++
>>> net/openvswitch/flow_netlink.c | 6 ++++++
>>> 2 files changed, 18 insertions(+)
>>>
>>> diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
>>> index 43f5dd7..c658d95 100644
>>> --- a/net/openvswitch/conntrack.h
>>> +++ b/net/openvswitch/conntrack.h
>>> @@ -34,6 +34,13 @@ int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
>>> void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
>>> int ovs_ct_put_key(const struct sw_flow_key *key, struct sk_buff *skb);
>>> void ovs_ct_free_action(const struct nlattr *a);
>>> +
>>> +static inline bool ovs_ct_state_supported(u8 state)
>>> +{
>>> + return !(state & ~(OVS_CS_F_NEW | OVS_CS_F_ESTABLISHED |
>>> + OVS_CS_F_RELATED | OVS_CS_F_REPLY_DIR |
>>> + OVS_CS_F_INVALID | OVS_CS_F_TRACKED));
>>> +}
>>> #else
>>> #include <linux/errno.h>
>>>
>>> @@ -46,6 +53,11 @@ static inline bool ovs_ct_verify(struct net *net, int attr)
>>> return false;
>>> }
>>>
>>> +static inline bool ovs_ct_state_supported(u8 state)
>>> +{
>>> + return false;
>>> +}
>>> +
>>> static inline int ovs_ct_copy_action(struct net *net, const struct nlattr *nla,
>>> const struct sw_flow_key *key,
>>> struct sw_flow_actions **acts, bool log)
>>> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
>>> index ea82cd5..c4917c9 100644
>>> --- a/net/openvswitch/flow_netlink.c
>>> +++ b/net/openvswitch/flow_netlink.c
>>> @@ -816,6 +816,12 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
>>> ovs_ct_verify(net, OVS_KEY_ATTR_CT_STATE)) {
>>> u8 ct_state = nla_get_u8(a[OVS_KEY_ATTR_CT_STATE]);
>>>
>> We also need to return error if kernel does not support the feature.
>
> Already handled by ovs_ct_verify() in conntrack.h.
Right. Patch looks good then.
Acked-by: Pravin B Shelar <pshelar@nicira.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web