On 1/23/26 1:08 PM, Dumitru Ceara wrote:
> 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.
>
Typo: "But that doesn't change 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