| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALnjE+rxFp1nnvCV4rZke0JwLrR9Pumo_DAoGTzznYj_Fm7ozg@mail.gmail.com>
Date: Fri, 2 Oct 2015 14:59:38 -0700
From: Pravin Shelar <pshelar@...ira.com>
To: Thomas F Herbert <thomasfherbert@...il.com>
Cc: netdev <netdev@...r.kernel.org>, therbert@...hat.com,
"dev@...nvswitch.org" <dev@...nvswitch.org>
Subject: Re: [PATCH net-next V14 3/3] openvswitch: 802.1ad: Flow handling,
actions, vlan parsing and netlink attributes
On Fri, Oct 2, 2015 at 2:48 PM, Thomas F Herbert
<thomasfherbert@...il.com> wrote:
> On 9/30/15 11:33 PM, Thomas F Herbert wrote:
>>
>> Add support for 802.1ad including the ability to push and pop double
>> tagged vlans. Add support for 802.1ad to netlink parsing and flow
>> conversion. Uses double nested encap attributes to represent double
>> tagged vlan. Inner TPID encoded along with ctci in nested attributes.
>>
>> Signed-off-by: Thomas F Herbert <thomasfherbert@...il.com>
>> ---
>> net/openvswitch/actions.c | 4 +-
>> net/openvswitch/flow.c | 87 +++++++++++++++++----
>> net/openvswitch/flow.h | 11 ++-
>> net/openvswitch/flow_netlink.c | 167
>> +++++++++++++++++++++++++++++++++++++----
>> net/openvswitch/vport-netdev.c | 4 +-
>> 5 files changed, 239 insertions(+), 34 deletions(-)
>>
...
>> diff --git a/net/openvswitch/flow_netlink.c
>> b/net/openvswitch/flow_netlink.c
>> index c92d6a2..08f56ab 100644
>> --- a/net/openvswitch/flow_netlink.c
>> +++ b/net/openvswitch/flow_netlink.c
>> @@ -811,6 +811,27 @@ static int metadata_from_nlattrs(struct net *net,
>> struct sw_flow_match *match,
>> return 0;
>> }
>> +static int cust_vlan_from_nlattrs(struct sw_flow_match *match,
>> + const struct nlattr *a[],
>> + bool is_mask, bool log)
>> +{
>> + __be16 ctci = 0;
>> + __be16 c_tpid = 0;
>> +
>> + ctci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
>> + if (!(ctci & htons(VLAN_TAG_PRESENT))) {
>> + if (is_mask)
>> + OVS_NLERR(log, "VLAN CTCI mask does not have exact
>> match for VLAN_TAG_PRESENT bit.");
>> + else
>> + OVS_NLERR(log, "VLAN CTCI does not have
>> VLAN_TAG_PRESENT bit set.");
>> + return -EINVAL;
>> + }
>> + c_tpid = nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]);
>> + SW_FLOW_KEY_PUT(match, eth.cvlan.tpid, c_tpid, is_mask);
>> + SW_FLOW_KEY_PUT(match, eth.cvlan.tci, ctci, is_mask);
>> + return 0;
>> +}
>> +
>> static int ovs_key_from_nlattrs(struct net *net, struct sw_flow_match
>> *match,
>> u64 attrs, const struct nlattr **a,
>> bool is_mask, bool log)
>> @@ -845,7 +866,7 @@ static int ovs_key_from_nlattrs(struct net *net,
>> struct sw_flow_match *match,
>> return -EINVAL;
>> }
>> - SW_FLOW_KEY_PUT(match, eth.tci, tci, is_mask);
>> + SW_FLOW_KEY_PUT(match, eth.vlan.tci, tci, is_mask);
>> attrs &= ~(1 << OVS_KEY_ATTR_VLAN);
>> }
>> @@ -1064,6 +1085,93 @@ static void mask_set_nlattr(struct nlattr *attr,
>> u8 val)
>> nlattr_set(attr, val, ovs_key_lens);
>> }
>> +static int parse_vlan_from_nlattrs(const struct nlattr **nla,
>> + struct sw_flow_match *match,
>> + u64 *key_attrs, bool *ie_valid,
>> + const struct nlattr **a, bool is_mask,
>> + bool log)
>> +{
>> + int err;
>> + const struct nlattr *encap;
>> +
>> + if (!is_mask) {
>> + u64 v_attrs = 0;
>> +
>> + err = parse_flow_nlattrs(*nla, a, &v_attrs, log);
>> + if (err)
>> + return err;
>> + /* Another encap attribute here indicates
>> + * the presence of a double tagged vlan.
>> + */
>> + if ((v_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) &&
>> +
>> eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]))) {
>> + if (!((v_attrs & (1ULL << OVS_KEY_ATTR_VLAN)) &&
>> + (v_attrs & (1ULL << OVS_KEY_ATTR_ENCAP)))) {
>> + OVS_NLERR(log, "Invalid Inner VLAN
>> frame");
>> + return -EINVAL;
>> + }
>> + SW_FLOW_KEY_PUT(match, eth.vlan.tpid,
>> +
>> nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]),
>> + is_mask);
>> + encap = a[OVS_KEY_ATTR_ENCAP];
>> + v_attrs &= ~(1 << OVS_KEY_ATTR_ENCAP);
>> +
>> + err = cust_vlan_from_nlattrs(match, a, is_mask,
>> log);
>> + if (err)
>> + return err;
>> + *ie_valid = true;
>> + *nla = encap;
>> +
>> + /* Insure that tci key attribute isn't
>> + * overwritten by encapsulated customer tci.
>> + * Ethertype is cleared because it is c_tpid.
>> + */
>> + v_attrs &= ~(1 << OVS_KEY_ATTR_VLAN);
>> + v_attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE);
>> + }
>> + *key_attrs |= v_attrs;
>> +
>> + if (*ie_valid) {
>> + err = parse_flow_nlattrs(*nla, a, key_attrs, log);
>> + if (err)
>> + return err;
>> + }
>> +
>> + } else {
>> + u64 mask_v_attrs = 0;
>> +
>> + err = parse_flow_mask_nlattrs(*nla, a, &mask_v_attrs,
>> log);
>> + if (err)
>> + return err;
>> +
>> + if (mask_v_attrs & 1 << OVS_KEY_ATTR_ENCAP) {
>> + if (!*ie_valid) {
>> + OVS_NLERR(log, "Encap mask attribute is
>> set for non-CVLAN frame.");
>> + err = -EINVAL;
>> + return err;
>> + }
>> + encap = a[OVS_KEY_ATTR_ENCAP];
>> + mask_v_attrs &= ~(1 << OVS_KEY_ATTR_ENCAP);
>> +
>> + err = cust_vlan_from_nlattrs(match, a, is_mask,
>> log);
>> + if (err)
>> + return err;
>> + *nla = encap;
>> +
>> + mask_v_attrs &= ~(1ULL << OVS_KEY_ATTR_VLAN);
>> + mask_v_attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE);
>> + }
>> +
>> + *key_attrs |= mask_v_attrs;
>> + if (*ie_valid) {
>
> Pravin, could you please review the above code encoding the netmask of the
> tpids. I am seeing a duplicate key (Type 6, ethertype key) discovered by
> parse_vlan_from_nlattrs() below which I noticed (chagrined I am) only after
> submitting this patch.
>
Sure, I'm halfway through the review. Once it finished I will post it.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists