| Message ID | 1392656174-14791-4-git-send-email-sw@simonwunderlich.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 02/17/14 17:56, Simon Wunderlich wrote: > MPLS labels may contain traffic control information, which should be > evaluated and used by the wireless subsystem if present. > > Also check for IEEE 802.21 which is always network control traffic. > > Signed-off-by: Simon Wunderlich<sw@simonwunderlich.de> > Signed-off-by: Mathias Kretschmer<mathias.kretschmer@fokus.fraunhofer.de> > --- > Changes to first version: > > * include linux/mpls.h, not the UAPI one > * change __constant_htons to htons > --- > net/wireless/util.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/net/wireless/util.c b/net/wireless/util.c > index d39c371..54956eb 100644 > --- a/net/wireless/util.c > +++ b/net/wireless/util.c > @@ -11,6 +11,7 @@ > #include<net/ip.h> > #include<net/dsfield.h> > #include<linux/if_vlan.h> > +#include<linux/mpls.h> > #include "core.h" > #include "rdev-ops.h" > > @@ -710,6 +711,29 @@ unsigned int cfg80211_classify8021d(struct sk_buff *skb, So does the name still covers what it is doing now or should we just callit cfg80211_classify_skb()? > return vlan_priority; > } > > + if (skb_headlen(skb)>= sizeof(struct ethhdr)) { > + struct ethhdr *eh = (struct ethhdr *)skb->data; > + struct mpls_label_stack mpls_tmp, *mpls; > + > + switch (eh->h_proto) { It seem eh->h_proto should be the same as skb->protocol, right? So why not add these case statements to the switch below? Regards, Arend > + case htons(ETH_P_MPLS_UC): > + case htons(ETH_P_MPLS_MC): > + /* MPLS */ > + mpls = skb_header_pointer(skb, sizeof(*eh), > + sizeof(*mpls),&mpls_tmp); > + if (!mpls) > + break; > + > + return (ntohl(mpls->entry)& MPLS_LS_TC_MASK) > + >> MPLS_LS_TC_SHIFT; > + case htons(ETH_P_80221): > + /* 802.21 is always network control traffic */ > + return 7; > + default: > + break; > + } > + } > + > switch (skb->protocol) { > case htons(ETH_P_IP): > dscp = ipv4_get_dsfield(ip_hdr(skb))& 0xfc; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> It seem eh->h_proto should be the same as skb->protocol, right? So why not > add these case statements to the switch below? Thought so, as well. But it seems that skb->protocol is not properly set when the packet originates from a packet socket. And in our case we cannot set it to a fixed etherType in the bind() call since we use three different etherTypes on the same socket. Cheers, Mathias > Regards, > Arend > > > + case htons(ETH_P_MPLS_UC): > > + case htons(ETH_P_MPLS_MC): > > + /* MPLS */ > > + mpls = skb_header_pointer(skb, sizeof(*eh), > > + sizeof(*mpls),&mpls_tmp); > > + if (!mpls) > > + break; > > + > > + return (ntohl(mpls->entry)& MPLS_LS_TC_MASK) > > + >> MPLS_LS_TC_SHIFT; > > + case htons(ETH_P_80221): > > + /* 802.21 is always network control traffic */ > > + return 7; > > + default: > > + break; > > + } > > + } > > + > > switch (skb->protocol) { > > case htons(ETH_P_IP): > > dscp = ipv4_get_dsfield(ip_hdr(skb))& 0xfc; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 02/17/14 17:56, Simon Wunderlich wrote: > > MPLS labels may contain traffic control information, which should be > > evaluated and used by the wireless subsystem if present. > > > > Also check for IEEE 802.21 which is always network control traffic. > > > > Signed-off-by: Simon Wunderlich<sw@simonwunderlich.de> > > Signed-off-by: Mathias Kretschmer<mathias.kretschmer@fokus.fraunhofer.de> > > --- > > > > Changes to first version: > > * include linux/mpls.h, not the UAPI one > > * change __constant_htons to htons > > > > --- > > > > net/wireless/util.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/net/wireless/util.c b/net/wireless/util.c > > index d39c371..54956eb 100644 > > --- a/net/wireless/util.c > > +++ b/net/wireless/util.c > > @@ -11,6 +11,7 @@ > > > > #include<net/ip.h> > > #include<net/dsfield.h> > > #include<linux/if_vlan.h> > > > > +#include<linux/mpls.h> > > > > #include "core.h" > > #include "rdev-ops.h" > > > > @@ -710,6 +711,29 @@ unsigned int cfg80211_classify8021d(struct sk_buff > > *skb, > > So does the name still covers what it is doing now or should we just > callit cfg80211_classify_skb()? I'm not completely sure why this function is called 8021d anyway (isn't 802.1D the Spanning Tree definition?). The actual QoS definitions are appearently in 802.1p. But I don't know both standards, so I'm just speculating. :) We can rename the function, but that would require more changes in drivers and mac80211. Anyway, I can resend the patch along with a rename, but would prefer to get the first two proposed patches merged first if there are no further objections. Cheers, Simon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Kretschmer, Mathias" <mathias.kretschmer@fokus.fraunhofer.de> Date: Mon, 17 Feb 2014 17:34:34 +0000 > Thought so, as well. But it seems that skb->protocol is not properly > set when the packet originates from a packet socket. And in our > case we cannot set it to a fixed etherType in the bind() call since > we use three different etherTypes on the same socket. So many things break if the skb->protocol isn't set correctly that I would consider such SKBs mal-formed. You can specify the protocol number in the msghdr passed into the send. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Thought so, as well. But it seems that skb->protocol is not properly > > set when the packet originates from a packet socket. And in our case > > we cannot set it to a fixed etherType in the bind() call since we use > > three different etherTypes on the same socket. > > So many things break if the skb->protocol isn't set correctly that I would > consider such SKBs mal-formed. Agreed. > You can specify the protocol number in the msghdr passed into the send. Will take a look. Thx. Need to check if there is also a way to set the protocol number when using TX_RINGs, where we use one send() call to signal the kernel that a bunch of frames (with potentially different protocol numbers) is ready for consumption. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Kretschmer, Mathias" <mathias.kretschmer@fokus.fraunhofer.de> Date: Mon, 17 Feb 2014 19:11:05 +0000 > Will take a look. Thx. Need to check if there is also a way to set > the protocol number when using TX_RINGs, where we use one send() > call to signal the kernel that a bunch of frames (with potentially > different protocol numbers) is ready for consumption. You will have to make seperate TX_RING sends() for each cluster where the protocol changes. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I see. OK. If that's the proper way to do it, then so be it. The performance hit should be negligible since to 99% it's all the same etherType. > -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Monday, February 17, 2014 20:14 > To: Kretschmer, Mathias > Cc: arend@broadcom.com; sw@simonwunderlich.de; > netdev@vger.kernel.org; linux-wireless@vger.kernel.org > Subject: Re: [net-next v2 3/3] cfg80211: add MPLS and 802.21 classification > > From: "Kretschmer, Mathias" <mathias.kretschmer@fokus.fraunhofer.de> > Date: Mon, 17 Feb 2014 19:11:05 +0000 > > > Will take a look. Thx. Need to check if there is also a way to set the > > protocol number when using TX_RINGs, where we use one send() call to > > signal the kernel that a bunch of frames (with potentially different > > protocol numbers) is ready for consumption. > > You will have to make seperate TX_RING sends() for each cluster where the > protocol changes. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/17/2014 08:06 PM, David Miller wrote: > From: "Kretschmer, Mathias" <mathias.kretschmer@fokus.fraunhofer.de> > Date: Mon, 17 Feb 2014 17:34:34 +0000 > >> Thought so, as well. But it seems that skb->protocol is not properly >> set when the packet originates from a packet socket. And in our >> case we cannot set it to a fixed etherType in the bind() call since >> we use three different etherTypes on the same socket. > > So many things break if the skb->protocol isn't set correctly that I > would consider such SKBs mal-formed. That was my take as well, but how does the kernel code consider this? > You can specify the protocol number in the msghdr passed into the > send. > Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/wireless/util.c b/net/wireless/util.c index d39c371..54956eb 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -11,6 +11,7 @@ #include <net/ip.h> #include <net/dsfield.h> #include <linux/if_vlan.h> +#include <linux/mpls.h> #include "core.h" #include "rdev-ops.h" @@ -710,6 +711,29 @@ unsigned int cfg80211_classify8021d(struct sk_buff *skb, return vlan_priority; } + if (skb_headlen(skb) >= sizeof(struct ethhdr)) { + struct ethhdr *eh = (struct ethhdr *)skb->data; + struct mpls_label_stack mpls_tmp, *mpls; + + switch (eh->h_proto) { + case htons(ETH_P_MPLS_UC): + case htons(ETH_P_MPLS_MC): + /* MPLS */ + mpls = skb_header_pointer(skb, sizeof(*eh), + sizeof(*mpls), &mpls_tmp); + if (!mpls) + break; + + return (ntohl(mpls->entry) & MPLS_LS_TC_MASK) + >> MPLS_LS_TC_SHIFT; + case htons(ETH_P_80221): + /* 802.21 is always network control traffic */ + return 7; + default: + break; + } + } + switch (skb->protocol) { case htons(ETH_P_IP): dscp = ipv4_get_dsfield(ip_hdr(skb)) & 0xfc;