On Thu, Dec 22, 2016 at 05:06:08PM +0530, Babu Shanmugam wrote:
> This patch attempts to avoid the usage of MAC_Binding table.
>
> Dynamic ARP response originates from the logical ports with "unknown"
> address. When the ARP resolution is requested via a logical
> router datapath, ARP response will be delivered to the logical router
> port of the switch. Since the logical router is distributed, the
> response will never reach the other chassis from where the ARP resolution
> is requested.
>
> This is done using a OVN logical action bcast2lr() which adds an action
> to send the packet to a specific multicast group for the logical router
> datapath. For the chassis that have an "unknown" port connected to any
> logical router, the bcast2lr() action will do the broadcast. The other
> chassis will listen for such broadcast packets and executes put_arp()
> action on them.
>
> When the ovn-controller processes put_arp() action, it caches the MAC
> bindings and this cache will be used to update the openflow table used
> for the dynamic MAC resolution
>
> Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
Thanks for the patch. We need a solution for this problem, and this is
the first real attempt I've seen.
Clang says:
../ovn/lib/actions.c:1672:9: error: variable 'dp_id' is used uninitialized
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:1699:23: note: uninitialized use occurs here
../ovn/lib/actions.c:1672:5: note: remove the 'if' if its condition is
always true
../ovn/lib/actions.c:1667:15: note: initialize the variable 'dp_id' to
silence this warning
../ovn/lib/actions.c:1672:9: error: variable 'chassis' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:1700:22: note: uninitialized use occurs here
../ovn/lib/actions.c:1672:5: note: remove the 'if' if its condition is
always true
../ovn/lib/actions.c:1669:18: note: initialize the variable 'chassis' to
silence this warning
../ovn/lib/actions.c:1672:9: error: variable 'port_key' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:1701:26: note: uninitialized use occurs here
../ovn/lib/actions.c:1672:5: note: remove the 'if' if its condition is
always true
../ovn/lib/actions.c:1668:18: note: initialize the variable 'port_key' to
silence this warning
../ovn/lib/actions.c:1672:9: error: variable 'is_this_chassis' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:1702:30: note: uninitialized use occurs here
../ovn/lib/actions.c:1672:5: note: remove the 'if' if its condition is
always true
../ovn/lib/actions.c:1670:25: note: initialize the variable
'is_this_chassis' to silence this warning
../ovn/lib/actions.c:1709:19: error: format specifies type 'long' but the
argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
../ovn/northd/ovn-northd.c:3144:25: error: format specifies type 'long' but
the argument has type 'int64_t' (aka 'long long') [-Werror,-Wformat]
../ovn/northd/ovn-northd.c:3145:25: error: format specifies type 'long' but
the argument has type 'int64_t' (aka 'long long') [-Werror,-Wformat]
GCC says:
../ovn/lib/actions.c: In function 'format_BCAST2LR':
../ovn/lib/actions.c:1708:41: error: format '%ld' expects argument of type
'long int', but argument 3 has type 'uint64_t {aka const long long unsigned
int}' [-Werror=format=]
ds_put_format(s, "bcast2lr(dst_dp=%ld, dst_pkey=%d, chassis=\"%s\");",
^
../ovn/lib/actions.c: In function 'parse_action':
../ovn/lib/actions.c:1702:28: error: 'is_this_chassis' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
bc2lr->is_this_chassis = is_this_chassis;
~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
../ovn/lib/actions.c:1670:10: note: 'is_this_chassis' was declared here
bool is_this_chassis;
^~~~~~~~~~~~~~~
../ovn/lib/actions.c:1700:20: error: 'chassis' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
bc2lr->chassis = chassis;
~~~~~~~~~~~~~~~^~~~~~~~~
../ovn/lib/actions.c:1669:11: note: 'chassis' was declared here
char *chassis;
^~~~~~~
../ovn/northd/ovn-northd.c: In function ‘build_lswitch_flows’:
../ovn/northd/ovn-northd.c:3142:44: error: format ‘%ld’ expects argument of
type ‘long int’, but argument 3 has type ‘int64_t {aka const long long int}’
[-Werror=format=]
"bcast2lr(dst_dp=%ld, dst_pkey=%ld,
chassis=\"%s\");"
^
../ovn/northd/ovn-northd.c:3142:58: error: format ‘%ld’ expects argument of
type ‘long int’, but argument 4 has type ‘int64_t {aka const long long int}’
[-Werror=format=]
"bcast2lr(dst_dp=%ld, dst_pkey=%ld,
chassis=\"%s\");"
I don't think that OVN logical actions should specify datapath or port
numbers; there are no precedents. Please use their names instead and
let ovn-controller translate.
Unfortunately, my biggest problem with this patch is that I simply don't
understand the description of how this whole thing is supposed to work.
Can you explain, step-by-step, how ARP now gets resolved?
Thanks,
Ben.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
