On Thu, Apr 7, 2016 at 9:48 PM, Justin Pettit <[email protected]> wrote:
> > > On Apr 6, 2016, at 11:26 PM, Numan Siddique <[email protected]> wrote: > > > > > > Thanks for the comments Justin. I tried a similar approach. It will not > work in the cases where the port security address also has a prefix defined. > > For example with port security - "00:00:00:00:00:02 10.0.0.4/24", the > ovn lexer parser is throwing the below error, > > > > ------- > > lflow|WARN|error parsing match "outport == "sw0-port2" && eth.dst == > 00:00:00:00:00:02 && ip4.dst == {255.255.255.255, 224.0.0.0/4, 10.0.0.4/24}": > Value contains unmasked 1-bits. > > ------ > > Ah, it should probably be added to the unit tests to make sure we don't > reintroduce a problem. (Thanks for writing unit tests, by the way.) > What if you apply the mask first like the patch at the end of this > message? I also expanded your unit tests to include a check for the issue > you mentioned. > > Hi Justin, there is still a problem with the below approach. In the case where port security has "10.0.0.4/24" it means that the logical port is restricted in sending and receiving IP traffic with ip address 10.0.0.4. IP traffic with any other ip address should be dropped. But with the below approach we would be allowing all the ip addresses in the 10.0.0.0/24. Below is the port security description <p> Each element in the set may additionally contain one or more IPv4 or IPv6 addresses (or both), with optional masks. If a mask is given, it must be a CIDR mask. In addition to the restrictions described for Ethernet addresses above, such an element restricts the IPv4 or IPv6 addresses from which the host may send and to which it may receive packets to the specified addresses. A masked address, if the host part is zero, indicates that the host is allowed to use any address in the subnet; if the host part is nonzero, the mask simply indicates the size of the subnet. In addition: </p> In the initial implementation I had missed to implement the case where the host part is zero. :) Thanks Numan --Justin > > > -=-=-=-=-=-=- > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 302cc1d..e60f72e 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1179,8 +1179,11 @@ build_port_security_nd(struct ovn_port *op, struct > hmap * > if (ps.n_ipv4_addrs) { > ds_put_cstr(&match, " && ("); > for (size_t i = 0; i < ps.n_ipv4_addrs; i++) { > - ds_put_format(&match, "arp.spa == "IP_FMT" || ", > - IP_ARGS(ps.ipv4_addrs[i].addr)); > + ovs_be32 mask = > be32_prefix_mask(ps.ipv4_addrs[i].plen); > + ds_put_cstr(&match, "arp.spa == "); > + ip_format_masked(ps.ipv4_addrs[i].addr & mask, mask, > + &match); > + ds_put_cstr(&match, " || "); > } > ds_chomp(&match, ' '); > ds_chomp(&match, '|'); > @@ -1264,7 +1267,9 @@ build_port_security_ip(enum ovn_pipeline pipeline, > struct > } > > for (int i = 0; i < ps.n_ipv4_addrs; i++) { > - ds_put_format(&match, IP_FMT", ", > IP_ARGS(ps.ipv4_addrs[i].addr > + ovs_be32 mask = be32_prefix_mask(ps.ipv4_addrs[i].plen); > + ip_format_masked(ps.ipv4_addrs[i].addr & mask, mask, > &match); > + ds_put_cstr(&match, ", "); > } > > /* Replace ", " by "}". */ > diff --git a/tests/ovn.at b/tests/ovn.at > index 22121e1..d8bc395 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1930,6 +1930,27 @@ for i in 1 2 3; do > test_ipv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip $tip > done > > +# configure lport13 to send and received IPv4 packets with an address > range > +ovn-nbctl lport-set-port-security lp13 "f0:00:00:00:00:13 192.168.0.13 > 10.0.0.4 > + > +sip=`ip_to_hex 10 0 0 14` > +tip=`ip_to_hex 192 168 0 23` > +# IPv4 packet from lport13 with src ip 10.0.0.14 destined to lport23 > +# with dst ip 192.168.0.23 should be allowed > +test_ip 13 f00000000013 f00000000023 $sip $tip 23 > + > +sip=`ip_to_hex 192 168 0 33` > +tip=`ip_to_hex 10 0 0 15` > +# IPv4 packet from lport33 with src ip 192.168.0.33 destined to lport13 > +# with dst ip 10.0.0.15 should be received by lport13 > +test_ip 33 f00000000033 f00000000013 $sip $tip 13 > + > +sip=`ip_to_hex 10 0 0 13` > +tip=`ip_to_hex 192 168 0 22` > +# arp packet with inner ip 10.0.0.13 should be allowed for lport13 > +test_arp 13 f00000000013 f00000000013 $sip $tip 0 f00000000022 > + > + > > # Allow some time for packet forwarding. > > > > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
