[ovs-dev] [PATCH] Modify the check for identfying STP BPDUs to avoid conflicts with IEEE 802.1QBG/LLDP

[Ben Pfaff]

Thanks for checking.  I applied this to master.

On Thu, Apr 24, 2014 at 01:12:27PM -0700, Padmanabhan Krishnan wrote:
> This works for my requirement of passing LLDP frames with NCB. Good to go from my side.
> 
> 
> Thanks,
> Paddu
> 
> On Thursday, April 24, 2014 11:16 AM, Padmanabhan Krishnan <kprad1 at yahoo.com> wrote:
>  
> Sure, this looks better. Wasn't aware of the define for FLOW_DL_TYPE_NONE. I was searching for the define for minimum ethertype. Now i do see ETH_TYPE_MIN. Let me apply this patch, test and will update this thread.
> 
> Thanks,
> Paddu
> 
> 
> Ben Pfaff?blp at nicira.com?
> Thu Apr 24 10:15:24 PDT 2014
> 	* Previous message:?[ovs-dev] [PATCH] Modify the check for identfying STP BPDUs to	avoid conflicts with IEEE 802.1QBG/LLDP
> 	* Next message:?[ovs-dev] [PATCH 1/3] daemon: Rename daemon.c as daemon-unix.c
> 	* Messages sorted by:?[ date ]?[ thread ]?[ subject ]?[ author ]
> ________________________________
> 
> On Thu, Apr 24, 2014 at 01:12:43AM -0700, Padmanabhan Krishnan wrote:
> >Apart from STP, EVB extension of LLDP as well as IEEE 802.1QBG >use the Nearest Customer Bridge (NCB) DMAC which >has a value of 0180.c200.0000. If a flow is programmed for >LLDP or QBG packets specifying the NCB DMAC and ethertype, >the userspace still drops the frame thinking it's a STP >frame. STP BPDU are not encoded in Ethernet II header, but >uses LLC encap and it should contain the length in the >respective field, instead of ethertype. So, the value will >be less than 0x800. An extra check is added to ensure frames with >>NCB DMAC and a valid etype are not treated as STP frames. >>>Signed-off-by: Padmanabhan Krishnan <http://openvswitch.org/mailman/listinfo/dev>  We can be more specific: any non-Ethernet II frame will have
> FLOW_DL_TYPE_NONE as its dl_type. Also, there are other bits of code that should change too. Here is a revised version.  Will you please verify that it also solves
> the problem for you? Thanks, Ben. --8<--------------------------cut here-------------------------->8-- From: Padmanabhan Krishnan <kprad1 at yahoo.com>
> Date: Thu, 24 Apr 2014 10:07:31 -0700
> Subject: [PATCH] ofproto-dpif-xlate: Identify STP BPDUs more specifically. Apart from STP, EVB extension of LLDP as well as IEEE 802.1QBG use the
> Nearest Customer Bridge (NCB) DMAC which has a value of 0180.c200.0000.
> STP can be distinguished by Ethertype from these protocols. Signed-off-by: Padmanabhan Krishnan <kprad1 at yahoo.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> --- lib/flow.h                   |    6 ++++++ ofproto/ofproto-dpif-xlate.c |    7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/flow.h b/lib/flow.h
> index 413ecb5..e63e4e2 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -617,4 +617,10 @@ static inline bool is_icmpv6(const struct flow *flow) && flow->nw_proto == IPPROTO_ICMPV6); } +static inline bool is_stp(const struct flow *flow)
> +{
> +    return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
> +            && flow->dl_type == htons(FLOW_DL_TYPE_NONE));
> +}
> + #endif /* flow.h */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 248382f..b69a48d 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -770,8 +770,9 @@ xport_stp_listen_state(const struct xport *xport) static bool stp_should_process_flow(const struct flow *flow, struct flow_wildcards *wc) {
> +    /* is_stp() also checks dl_type, but dl_type is always set in 'wc'. */ memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
> -    return eth_addr_equals(flow->dl_dst, eth_addr_stp);
> +    return is_stp(flow); } static void
> @@ -1820,7 +1821,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, xlate_report(ctx, "OFPPC_NO_FWD set, skipping output"); return; } else if (check_stp) {
> -        if (eth_addr_equals(ctx->base_flow.dl_dst, eth_addr_stp)) {
> +        if (is_stp(&ctx->base_flow)) { if (!xport_stp_listen_state(xport)) { xlate_report(ctx, "STP not in listening state, " "skipping bpdu output");
> @@ -2717,7 +2718,7 @@ xlate_sample_action(struct xlate_ctx *ctx, static bool may_receive(const struct xport *xport, struct xlate_ctx *ctx) {
> -    if (xport->config & (eth_addr_equals(ctx->xin->flow.dl_dst, eth_addr_stp)
> +    if (xport->config & (is_stp(&ctx->xin->flow) ? OFPUTIL_PC_NO_RECV_STP : OFPUTIL_PC_NO_RECV)) { return false;
> -- 
> 1.7.10.4 
> Thanks,
> PAddu