On 1/23/26 12:53 PM, Ilya Maximets wrote:
> On 1/23/26 11:38 AM, Dumitru Ceara wrote:
>> Hi all,
>>
>> Thanks for the patch and discussion! I'll add my own opinion below.
>>
>> On 1/23/26 7:30 AM, Ales Musil wrote:
>>> On Thu, Jan 22, 2026 at 7:40 PM Mark Michelson <[email protected]> wrote:
>>>
>>>> In the commit title and description, "compliant" is misspelled as
>>>> "complaint". It is spelled correctly in the code, though.
>>>>
>>>> I have comments below, both regarding the code submission and to
>>>> respond to Ilya's remarks.
>>>>
>>>> On Thu, Jan 22, 2026 at 12:18 PM Ilya Maximets <[email protected]> wrote:
>>>>>
>>>>> On 1/22/26 3:27 PM, Ales Musil via dev wrote:
>>>>>> The RFC defines a Virtual Router Redundancy Protocol [0], in order
>>>>>> for that protocol to work the workload might "spoof" MAC address
>>>>>> within ARP or ND request/response. This wasn't allowed as the port
>>>>>> security is specifically designed against spoofing and checks if
>>>>>> the port security MAC address is the same for source of ARP/ND
>>>>>> and the inner source/target address. To make the port security
>>>>>> complaint add an option which when enabled will add extra flows
>>>>>> that match on the MAC address range specified by the RFC.
>>>>>>
>>>>>> [0] https://datatracker.ietf.org/doc/html/rfc5798
>>>>>> Reported-at: https://issues.redhat.com/browse/FDP-2979
>>>>>> Signed-off-by: Ales Musil <[email protected]>
>>>>>> ---
>>>>>> NEWS | 2 +
>>>>>> controller/lflow.c | 67 ++++++++++++++++++++
>>>>>> ovn-nb.xml | 9 +++
>>>>>> tests/ovn.at | 149
>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 4 files changed, 227 insertions(+)
>>>>>>
>>>>>> diff --git a/NEWS b/NEWS
>>>>>> index dc9d28f2c..e58f8ee1e 100644
>>>>>> --- a/NEWS
>>>>>> +++ b/NEWS
>>>>>> @@ -87,6 +87,8 @@ Post v25.09.0
>>>>>> other_config column.
>>>>>> - Introduce the capability to specify multiple ips for
>>>> ovn-evpn-local-ip
>>>>>> option.
>>>>>> + - Add an LSP option called "port-security-rfc5798-compliant", this
>>>>>> + allows VRRP v3 (RFC 5798) to work with port security enabled.
>>>>>
>>>>> Hi, Ales.
>>>>>
>>>>> This option name seems a bit misleading, as someone seeing it may assume
>>>>> that just enbaling it will allow the port to participate in VRRPv3. But
>>>>> it's not the case as port security will drop all the actual VRRP traffic,
>>>>> unless the virtual router mac address is also added to the port security,
>>>>> as all the actual VRRP packets with have this address as a source. As
>>>> per
>>>>> RFC itself:
>>>>>
>>>>> Note: VRRP packets are transmitted with the virtual router MAC
>>>>> address as the source MAC address to ensure that learning bridges
>>>>> correctly determine the LAN segment the virtual router is
>>>>> attached to.
>>>>>
>>>>> We should either add explicit indication that this option only affects
>>>>> ARP/ND packets, which would still be a bit confusing and will make the
>>>>> name even longer, or just have one knob like port-security-allow-vrrpv3
>>>>> that also allows all the actual VRRP traffic, so users don't have to
>>>>> add all the possible router IDs into port security. This option may
>>>>> potentially accept a list of allowed router IDs or "yes" for all, but
>>>>> that can be added later if necessary.
>>>>
>>>> If this is the case, then does this mean that non-ARP and non-ND VRRP
>>>> traffic will still be dropped by port security flows when this option
>>>> is enabled? If so, then it seems like allowing the ARPs and NDs is
>>>> only getting rid of the surface issue. The ARPs/NDs will be allowed
>>>> through, but then other VRRP traffic will still be dropped. Is that
>>>> OK? It seems like we're going to immediately get a complaint about
>>>> this patch not actually fixing the problem if that's the case. I think
>>>> Ilya is on the right track by suggesting that we allow all VRRP
>>>> traffic with this option, not just the ARP/ND packets.
>>>>
>>
>> The bug report was about ARP replies not being allowed. VRRP ARP
>> replies for the virtual IP are special:
>>
>> https://datatracker.ietf.org/doc/html/rfc5798#section-8.1.2
>>
>> When a host sends an ARP request for one of the virtual router IPv4
>> addresses, the Virtual Router Master MUST respond to the ARP request
>> with an ARP response that indicates the virtual MAC address for the
>> virtual router. Note that the source address of the Ethernet frame
>> of this ARP response is the physical MAC address of the physical
>> router.
>>
>> The traffic the virtual router actually forwards, IP traffic, is sent
>> using the _virtual_ MAC as source, i.e., from 00-00-5E-00-01-{VRID} (for
>> IPv4).
>>
>> But as the RFC states, the ARP replies are sent from with eth.src ==
>> _physical_ MAC of the VRRP member and arp.sha == 00-00-5E-00-01-{VRID}.
>>
>> Actual VRRP protocol packets are multicast and use the _physical_ MAC
>> address as source.
>
> I suppose, you meant "virtual" here. See the note at the end of
> https://datatracker.ietf.org/doc/html/rfc5798#section-7.2 that I
> cited before.
>
You're right, it should be "virtual". But that doesn't my argument
above much.
>>
>> So, in order for ALL this to work, the CMS must _anyway_ configure two
>> MACs (physical and 00-00-5E-00-01-{VRID}) in the port_security field of
>> the LSP.
>
> That is true. But why not save users from extra configuration and
> just allow the new knob to allow both the ARP/ND and the VRRP and
> the routed traffic? AFAIU, anyone setting the option will always
> have to set the MACs in the port_security column as well. There is
> no point in setting the option and not adding virtual MAC to the
> port_security list as well. So I'm not sure why we're forcing users
> to do that when we already adding a special vrrp knob.
>
Your suggestion was:
>>>>> We should either add explicit indication that this option
>>>>> only affects ARP/ND packets, which would still be a bit
>>>>> confusing and will make the name even longer, or just have
>>>>> one knob like port-security-allow-vrrpv3 that also allows
>>>>> all the actual VRRP traffic, so users don't have to add all
>>>>> the possible router IDs into port security. This option may
>>>>> potentially accept a list of allowed router IDs or "yes" for
>>>>> all, but that can be added later if necessary.
I read it as:
1. for now support "yes/no" for this option.
2. if "yes" then the user needs to configure the physical MAC in the
port_security field.
In that case OVN will allow:
- all traffic from/to the physical MAC, including VRRP ARPs with arp.sha
== VRRP-MAC (all VRIDs)
but also
- all traffic from/to any VRRP VID mac, i.e, any 00-00-5E-00-01-{VRID}
This is less "secure" than the additional knob added by Ales in this
patch which, in combination with explicitly adding the user VRID MAC to
port_security along with the physical MAC, would allow routed IP traffic
only if it's routed by that explicitly configured VRRP VRID.
Now, if we assume in the future we allow a list of VRRP MACs instead of
"yes" as value to the option, the user still needs to configure:
- port_security=[<physical-MAC>]
- port-security-allow-vrrpv3=[<VRRP-MAC1>, <VRRP-MAC2>]
While that's one config less, it does mean that we're not as restrictive
as we could be in 26.03 (or until we backport the extended config support).
>>
>> The problem is that port security semantics in OVN are too strict and
>> don't allow the mismatch between physical MAC and inner VRRP MAC for
>> ARPs/ND.
>>
>> So the knob is _only_ about arp/nd VRRP RFC 5798 compliance, in my opinion.
>>
>>>
>>> It is specifically said in the documentation that this applies
>>> only to ARP/ND, the user is expected to configure all MACs they
>>> want to allow in port security as usual. We can change the name of
>>> the option to make it more explicit. Also the regular traffic doesn't
>>> have any issues if it's configured properly, this option is there for
>>> the only part that cannot be configured in any way now.
>>>
>>
>> Maybe "port-security-rfc5798-arp-nd-compliant"? It's long but it's
>> explicit enough. We could also do "port-security-vrrp-arp-nd-compliant"
>> I guess.
>>
>> Regards,
>> Dumitru
>>
>>>
>>>>>
>>>>> I didn't read too deep into the code, but see a few comments inline.
>>>>>
>>>>> Best rgeards, Ilya Maximets.
>>>>>
>>>>>>
>>>>>> OVN v25.09.0 - xxx xx xxxx
>>>>>> --------------------------
>>>>>> diff --git a/controller/lflow.c b/controller/lflow.c
>>>>>> index b0998e605..93d9d090e 100644
>>>>>> --- a/controller/lflow.c
>>>>>> +++ b/controller/lflow.c
>>>>>> @@ -2551,6 +2551,11 @@ build_in_port_sec_arp_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> return;
>>>>>> }
>>>>>>
>>>>>> + bool vrrp_compliant =
>>>>>> + smap_get_bool(&pb->options,
>>>> "port-security-rfc5798-compliant", false);
>>>>>> + const struct eth_addr vrrp_mac = ETH_ADDR_C(00,00,5e,00,01,00);
>>>>
>>>> Nit: This and all the other const eth_addrs should be "static" as well.
>>>>
>>>>>> + const struct eth_addr vrrp_mask = ETH_ADDR_C(ff,ff,ff,ff,ff,00);
>>>>>> +
>>>>>> build_port_sec_allow_action(ofpacts);
>>>>>>
>>>>>> if (!ps_addr->n_ipv4_addrs) {
>>>>>> @@ -2561,6 +2566,9 @@ build_in_port_sec_arp_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> * match - "inport == pb->port && eth.src == ps_addr.ea &&
>>>>>> * arp && arp.sha == ps_addr.ea"
>>>>>> * action - "port_sec_failed = 0;"
>>>>>> + * If port security is configured to be VRRP (RFC5798)
>>>> compliant add
>>>>>
>>>>> Just the ARP flows do not make it RFC-compliant.
>>>>>
>>>>>> + * an extra flow to match on
>>>>>> + * arp.sha == 00:00:5e:00:01:00/ff:ff:ff:ff:ff:00
>>>>>> */
>>>>>> reset_match_for_port_sec_flows(pb, MFF_LOG_INPORT, m);
>>>>>> match_set_dl_src(m, ps_addr->ea);
>>>>>> @@ -2569,6 +2577,13 @@ build_in_port_sec_arp_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>> +
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_sha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND,
>>>> 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> }
>>>>>>
>>>>>> /* Add the below logical flow equivalent OF rules in
>>>> 'in_port_sec_nd'
>>>>>> @@ -2577,6 +2592,9 @@ build_in_port_sec_arp_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> * match - "inport == pb->port && eth.src == ps_addr.ea &&
>>>>>> * arp && arp.sha == ps_addr.ea && arp.spa ==
>>>> {ps_addr.ipv4_addrs}"
>>>>>> * action - "port_sec_failed = 0;"
>>>>>> + * If port security is configured to be VRRP (RFC5798) compliant
>>>> add
>>>>>> + * an extra flow to match on
>>>>>> + * arp.sha == 00:00:5e:00:01:00/ff:ff:ff:ff:ff:00
>>>>>> */
>>>>>> for (size_t j = 0; j < ps_addr->n_ipv4_addrs; j++) {
>>>>>> reset_match_for_port_sec_flows(pb, MFF_LOG_INPORT, m);
>>>>>> @@ -2594,6 +2612,13 @@ build_in_port_sec_arp_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>> +
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_sha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND,
>>>> 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> @@ -2701,6 +2726,11 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> struct match *m, struct ofpbuf *ofpacts,
>>>>>> struct ovn_desired_flow_table *flow_table)
>>>>>> {
>>>>>> + bool vrrp_compliant =
>>>>>> + smap_get_bool(&pb->options,
>>>> "port-security-rfc5798-compliant", false);
>>>>>> + const struct eth_addr vrrp_mac = ETH_ADDR_C(00,00,5e,00,02,00);
>>>>>> + const struct eth_addr vrrp_mask = ETH_ADDR_C(ff,ff,ff,ff,ff,00);
>>>>>> +
>>>>>> build_port_sec_allow_action(ofpacts);
>>>>>>
>>>>>> /* Add the below logical flow equivalent OF rules in
>>>> 'in_port_sec_nd'
>>>>>> @@ -2710,6 +2740,9 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> * icmp6 && icmp6.code == 135 && icmp6.type == 0 &&
>>>>>> * ip6.tll == 255 && nd.sll == {00:00:00:00:00:00,
>>>> ps_addr.ea}"
>>>>>> * action - "port_sec_failed = 0;"
>>>>>> + * If port security is configured to be VRRP (RFC5798) compliant
>>>> add
>>>>>> + * an extra flow to match on
>>>>>> + * nd.sll == 00:00:5e:00:02:00/ff:ff:ff:ff:ff:00
>>>>>> */
>>>>>> reset_match_for_port_sec_flows(pb, MFF_LOG_INPORT, m);
>>>>>> match_set_dl_type(m, htons(ETH_TYPE_IPV6));
>>>>>> @@ -2728,6 +2761,13 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>>
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_sha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> +
>>>>>> match_set_icmp_type(m, 136);
>>>>>> match_set_icmp_code(m, 0);
>>>>>> if (ps_addr->n_ipv6_addrs) {
>>>>>> @@ -2739,6 +2779,9 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> * nd.tll == {00:00:00:00:00:00, ps_addr.ea} &&
>>>>>> * nd.target == {ps_addr.ipv6_addrs, lla}"
>>>>>> * action - "port_sec_failed = 0;"
>>>>>> + * If port security is configured to be VRRP (RFC5798)
>>>> compliant add
>>>>>> + * an extra flow to match on
>>>>>> + * nd.tll == 00:00:5e:00:02:00/ff:ff:ff:ff:ff:00
>>>>>> */
>>>>>> struct in6_addr lla;
>>>>>> in6_generate_lla(ps_addr->ea, &lla);
>>>>>> @@ -2754,6 +2797,13 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>>
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_tha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND,
>>>> 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> +
>>>>>> for (size_t j = 0; j < ps_addr->n_ipv6_addrs; j++) {
>>>>>> reset_match_for_port_sec_flows(pb, MFF_LOG_INPORT, m);
>>>>>> match_set_dl_src(m, ps_addr->ea);
>>>>>> @@ -2781,6 +2831,13 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND,
>>>> 90,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>> +
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_tha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table,
>>>> OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> }
>>>>>> } else {
>>>>>> /* Add the below logical flow equivalent OF rules in
>>>> 'in_port_sec_nd'
>>>>>> @@ -2790,6 +2847,9 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> * icmp6.code == 136 && icmp6.type == 0 && ip6.tll
>>>> == 255 &&
>>>>>> * nd.tll == {00:00:00:00:00:00, ps_addr.ea}"
>>>>>> * action - "port_sec_failed = 0;"
>>>>>> + * If port security is configured to be VRRP (RFC5798)
>>>> compliant add
>>>>>> + * extra an flow to match on
>>>>>> + * nd.tll == 00:00:5e:00:02:00/ff:ff:ff:ff:ff:00
>>>>>> */
>>>>>> match_set_arp_tha(m, eth_addr_zero);
>>>>>> ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> @@ -2800,6 +2860,13 @@ build_in_port_sec_nd_flows(const struct
>>>> sbrec_port_binding *pb,
>>>>>> ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND, 90,
>>>>>> pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> &pb->header_.uuid);
>>>>>> +
>>>>>> + if (vrrp_compliant) {
>>>>>> + match_set_arp_tha_masked(m, vrrp_mac, vrrp_mask);
>>>>>> + ofctrl_add_flow(flow_table, OFTABLE_CHK_IN_PORT_SEC_ND,
>>>> 90,
>>>>>> + pb->header_.uuid.parts[0], m, ofpacts,
>>>>>> + &pb->header_.uuid);
>>>>>> + }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>>>>> index e74c0d010..4e95a13b6 100644
>>>>>> --- a/ovn-nb.xml
>>>>>> +++ b/ovn-nb.xml
>>>>>> @@ -1674,6 +1674,15 @@
>>>>>> <ref table="Interface" db="vswitch"/> table. This in
>>>> turn will
>>>>>> make OVS vswitchd update the MTU of the linked interface.
>>>>>> </column>
>>>>>> +
>>>>>> + <column name="options" key="port-security-rfc5798-compliant"
>>>>>> + type='{"type": "boolean"}'>
>>>>>> + Allows the inner ARP SHA or ND TLL/SLL to be within the
>>>> range of
>>>>>> + MAC addresses specified by RFC 5798. The result is a
>>>> masked match
>>>>>> + that allows the whole range of MAC addresses to pass
>>>> through.
>>>>>> + This option is only applicable if <ref
>>>> column="port_security"/>
>>>>>> + is populated.
>>>>>> + </column>
>>>>>> </group>
>>>>>> </group>
>>>>>>
>>>>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>>>>> index d5ee90e17..4455b7580 100644
>>>>>> --- a/tests/ovn.at
>>>>>> +++ b/tests/ovn.at
>>>>>> @@ -44181,3 +44181,152 @@ check ovn-nbctl --wait=hv lsp-set-type
>>>> down_ext localnet
>>>>>> OVN_CLEANUP([hv1],[hv2])
>>>>>> AT_CLEANUP
>>>>>> ])
>>>>>> +
>>>>>> +OVN_FOR_EACH_NORTHD([
>>>>>> +AT_SETUP([Port security - RFC 5798])
>>>>>> +AT_SKIP_IF([test $HAVE_SCAPY = no])
>>>>>> +ovn_start
>>>>>> +net_add n1
>>>>>> +sim_add hv1
>>>>>> +as hv1
>>>>>> +ovs-vsctl add-br br-phys
>>>>>
>>>>> check
>>>>>
>>>>>> +ovn_attach n1 br-phys 192.168.0.1
>>>>>> +
>>>>>> +check ovn-nbctl ls-add ls
>>>>>> +
>>>>>> +check ovn-nbctl lsp-add ls lsp1
>>>>>> +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:10:01
>>>> 192.168.10.1 fd10::1"
>>>>>> +
>>>>>> +check ovn-nbctl lsp-add ls lsp2
>>>>>> +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:10:02
>>>> 192.168.10.2 fd10::2"
>>>>>> +
>>>>>> +check ovs-vsctl -- add-port br-int vif1 -- \
>>>>>> + set interface vif1 external-ids:iface-id=lsp1 \
>>>>>> + options:tx_pcap=hv1/vif1-tx.pcap \
>>>>>> + options:rxq_pcap=hv1/vif1-rx.pcap
>>>>>> +
>>>>>> +check ovs-vsctl -- add-port br-int vif2 -- \
>>>>>> + set interface vif2 external-ids:iface-id=lsp2 \
>>>>>> + options:tx_pcap=hv1/vif2-tx.pcap \
>>>>>> + options:rxq_pcap=hv1/vif2-rx.pcap
>>>>>> +
>>>>>> +wait_for_ports_up
>>>>>> +
>>>>>> +test_arp() {
>>>>>> + local dropped=$1
>>>>>> +
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='ff:ff:ff:ff:ff:ff', src='00:00:00:00:10:01') /
>>>>>> + ARP(op=1, hwsrc='00:00:5e:00:01:05',
>>>> hwdst='ff:ff:ff:ff:ff:ff', psrc='192.168.10.1', pdst='192.168.10.2')
>>>>>> + ")
>>>>>> + as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
>>>>>
>>>>> Need to check that this actually worked.
>>>>>
>>>>>> +
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='ff:ff:ff:ff:ff:ff', src='00:00:00:00:10:01') /
>>>>>> + ARP(op=2, hwsrc='00:00:5e:00:01:05',
>>>> hwdst='ff:ff:ff:ff:ff:ff', psrc='192.168.10.1', pdst='192.168.10.1')
>>>>>> + ")
>>>>>
>>>>> Indentation is different here for some reason.
>>>>>
>>>>>> + as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
>>>>>
>>>>> check
>>>>>
>>>>>> +
>>>>>> + if [[ "$dropped" != "yes" ]]; then
>>>>>> + echo $packet >> vif2.expected
>>>>>
>>>>> Why are we sending two packets, but expecting only the second one?
>>>>
>>>> The first ARP has pdst not equal to lsp1's IP address, so it's
>>>> expected to be dropped by port security, even if VRRP compliance is
>>>> enabled. It's a control to ensure that a packet that should always be
>>>> dropped is not slipping through when VRRP compliance is enabled. The
>>>> second ARP has pdst equal to lsp1's IP address, so it is expected to
>>>> be dropped only if VRRP compliance is disabled.
>>>>
>>>
>>> We are sending ARP and gARP, ARP will be replied by LS lflows
>>> so it will never arrive to the second port, gARP will go through
>>> that's why we expected one packet for each port. One ARP reply
>>> and second gARP. The same happens with ND packets.
>>>
>>>
>>>>
>>>>>
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='00:00:00:00:10:01', src='00:00:00:00:10:02') /
>>>>>> + ARP(op=2, hwsrc='00:00:00:00:10:02',
>>>> hwdst='00:00:5e:00:01:05', psrc='192.168.10.2', pdst='192.168.10.1')
>>>>>> + ")
>>>>>> + echo $packet >> vif1.expected
>>>>>> + fi
>>>>>> +}
>>>>>> +test_nd() {
>>>>>> + local dropped=$1
>>>>>> +
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='33:33:ff:ff:ff:ff', src='00:00:00:00:10:01') /
>>>>>
>>>>> This is not a correct IPv6 multicast address.
>>>>>
>>>>>> + IPv6(src='fd10::1', dst='ff02::1:ff00:2') /
>>>>>> + ICMPv6ND_NS(tgt='fd10::2') /
>>>>>> + ICMPv6NDOptSrcLLAddr(lladdr='00:00:5e:00:02:05')
>>>>>> + ")
>>>>>> + as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
>>>>>
>>>>> check
>>>>>
>>>>>> +
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='ff:ff:ff:ff:ff:ff', src='00:00:00:00:10:01') /
>>>>>
>>>>> Address.
>>>>>
>>>>>> + IPv6(src='fd10::1', dst='ff01::1') /
>>>>>> + ICMPv6ND_NA(tgt='fd10::1') /
>>>>>> + ICMPv6NDOptDstLLAddr(lladdr='00:00:5e:00:02:05')
>>>>>> + ")
>>>>>> + as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
>>>>>
>>>>> check
>>>>>
>>>>>> +
>>>>>> + if [[ "$dropped" != "yes" ]]; then
>>>>>> + echo $packet >> vif2.expected
>>>>>> +
>>>>>> + packet=$(fmt_pkt "
>>>>>> + Ether(dst='00:00:00:00:10:01',
>>>> src='00:00:00:00:10:02') /
>>>>>> + IPv6(src='fd10::2', dst='fd10::1') /
>>>>>> + ICMPv6ND_NA(tgt='fd10::2', R=0, S=1, O=1) /
>>>>>> + ICMPv6NDOptDstLLAddr(lladdr='00:00:00:00:10:02')
>>>>>> + ")
>>>>>> + echo $packet >> vif1.expected
>>>>>> + fi
>>>>>> +}
>>>>>> +
>>>>>> +reset_pcap_and_expected() {
>>>>>> + reset_pcap_file vif1 hv1/vif1
>>>>>> + reset_pcap_file vif2 hv1/vif2
>>>>>> +
>>>>>> + : > vif1.expected
>>>>>> + : > vif2.expected
>>>>>> +}
>>>>>> +
>>>>>> +AS_BOX([Without port security])
>>>>>> +reset_pcap_and_expected
>>>>>> +
>>>>>> +test_arp no
>>>>>> +test_nd no
>>>>>> +
>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected])
>>>>>> +
>>>>>> +AS_BOX([With MAC only port security])
>>>>>> +reset_pcap_and_expected
>>>>>> +check ovn-nbctl --wait=hv lsp-set-port-security lsp1
>>>> "00:00:00:00:10:01"
>>>>>> +
>>>>>> +test_arp yes
>>>>>> +test_nd yes
>>>>>> +
>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected])
>>>>>> +
>>>>>> +AS_BOX([With MAC + IP port security])
>>>>>> +reset_pcap_and_expected
>>>>>> +check ovn-nbctl --wait=hv lsp-set-port-security lsp1
>>>> "00:00:00:00:10:01 192.168.10.1 fd10::1"
>>>>>> +
>>>>>> +test_arp yes
>>>>>> +test_nd yes
>>>>>> +
>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected])
>>>>>> +
>>>>>> +AS_BOX([With MAC only port security, compatibility enabled])
>>>>>> +reset_pcap_and_expected
>>>>>> +check ovn-nbctl lsp-set-options lsp1
>>>> port-security-rfc5798-compliant=true
>>>>>> +check ovn-nbctl --wait=hv lsp-set-port-security lsp1
>>>> "00:00:00:00:10:01"
>>>>>> +
>>>>>> +test_arp no
>>>>>> +test_nd no
>>>>>> +
>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected])
>>>>>> +
>>>>>> +AS_BOX([With MAC + IP port security, compatibility enabled])
>>>>>> +reset_pcap_and_expected
>>>>>> +check ovn-nbctl --wait=hv lsp-set-port-security lsp1
>>>> "00:00:00:00:10:01 192.168.10.1 fd10::1"
>>>>>> +
>>>>>> +test_arp no
>>>>>> +test_nd no
>>>>>> +
>>>>>> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>>>>>> +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected])
>>>>>> +
>>>>>> +OVN_CLEANUP([hv1])
>>>>>> +
>>>>>> +AT_CLEANUP
>>>>>> +])
>>>>>
>>>>> _______________________________________________
>>>>> dev mailing list
>>>>> [email protected]
>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>>
>>>>
>>>>
>>>
>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
