[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-10 Thread ganeshmurthy
Github user ganeshmurthy closed the pull request at:

https://github.com/apache/qpid-dispatch/pull/185


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ganeshmurthy
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132308131
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -504,6 +504,13 @@
 "deprecated": true,
 "description": "(DEPRECATED) This value is no longer 
used in the router.",
 "create": true
+},
+"defaultDistribution": {
+"type": ["multicast", "closest", "balanced", 
"forbidden"],
+"description": "Default forwarding treatment for any 
address without a specified treatment. multicast - one copy of each message 
delivered to all subscribers; closest - messages delivered to only the closest 
subscriber; balanced - messages delivered to one subscriber with load balanced 
across subscribers; linkBalanced - for link-routing, link attaches balanced 
across destinations; forbidden - this address is forbidden, link attaches to an 
address of forbidden distribution will be rejected.",
--- End diff --

Removed linkBalanced from comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread grs
Github user grs commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132281323
  
--- Diff: src/router_core/transfer.c ---
@@ -634,6 +634,23 @@ static void qdr_link_forward_CT(qdr_core_t *core, 
qdr_link_t *link, qdr_delivery
 addr->deliveries_ingress++;
 link->total_deliveries++;
 }
+//
+// There is no address that we can send this delivery to, which means 
the addr was not found in our hastable. This
+// can be because there were no receivers or because the address was 
not defined in the config file.
+// If the treatment for such addresses is set to be forbidden, we send 
back a rejected disposition and detach the link
+//
+else if (core->qd->treatment == QD_TREATMENT_LINK_FORBIDDEN) {
+dlv->disposition = PN_REJECTED;
+dlv->error = qdr_error("qd:forbidden", "Sending deliveries to this 
address is forbidden");
--- End diff --

Or perhaps even better would be amqp:not-found, since we are really saying 
that this is not a recognised address?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread grs
Github user grs commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132280889
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -504,6 +504,13 @@
 "deprecated": true,
 "description": "(DEPRECATED) This value is no longer 
used in the router.",
 "create": true
+},
+"defaultDistribution": {
+"type": ["multicast", "closest", "balanced", 
"forbidden"],
+"description": "Default forwarding treatment for any 
address without a specified treatment. multicast - one copy of each message 
delivered to all subscribers; closest - messages delivered to only the closest 
subscriber; balanced - messages delivered to one subscriber with load balanced 
across subscribers; linkBalanced - for link-routing, link attaches balanced 
across destinations; forbidden - this address is forbidden, link attaches to an 
address of forbidden distribution will be rejected.",
--- End diff --

linkBalanced should be removed from the comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ganeshmurthy
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132275399
  
--- Diff: src/dispatch_private.h ---
@@ -52,6 +52,7 @@ struct qd_dispatch_t {
 qd_connection_manager_t *connection_manager;
 qd_policy_t *policy;
 void*dl_handle;
+qd_address_treatment_t   treatment;
--- End diff --

Changed treatment to default_treatment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ganeshmurthy
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132275310
  
--- Diff: include/qpid/dispatch/router.h ---
@@ -42,7 +42,8 @@ typedef enum {
 QD_TREATMENT_MULTICAST_ONCE   = 1,
 QD_TREATMENT_ANYCAST_CLOSEST  = 2,
 QD_TREATMENT_ANYCAST_BALANCED = 3,
-QD_TREATMENT_LINK_BALANCED= 4
+QD_TREATMENT_LINK_BALANCED= 4,
+QD_TREATMENT_LINK_FORBIDDEN   = 5
--- End diff --

Changed QD_TREATMENT_LINK_FORBIDDEN to QD_TREATMENT_FORBIDDEN


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ted-ross
Github user ted-ross commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132259033
  
--- Diff: include/qpid/dispatch/router.h ---
@@ -42,7 +42,8 @@ typedef enum {
 QD_TREATMENT_MULTICAST_ONCE   = 1,
 QD_TREATMENT_ANYCAST_CLOSEST  = 2,
 QD_TREATMENT_ANYCAST_BALANCED = 3,
-QD_TREATMENT_LINK_BALANCED= 4
+QD_TREATMENT_LINK_BALANCED= 4,
+QD_TREATMENT_LINK_FORBIDDEN   = 5
--- End diff --

This constant should not have "_LINK_" in it.  That implies that this is 
only for link-routing.  You should rename it "QD_TREATMENT_FORBIDDEN" because 
this feature is not relevant for link-routing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ted-ross
Github user ted-ross commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132259545
  
--- Diff: src/dispatch_private.h ---
@@ -52,6 +52,7 @@ struct qd_dispatch_t {
 qd_connection_manager_t *connection_manager;
 qd_policy_t *policy;
 void*dl_handle;
+qd_address_treatment_t   treatment;
--- End diff --

Shouldn't this be called default_treatment?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ted-ross
Github user ted-ross commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132261405
  
--- Diff: src/router_core/transfer.c ---
@@ -634,6 +634,23 @@ static void qdr_link_forward_CT(qdr_core_t *core, 
qdr_link_t *link, qdr_delivery
 addr->deliveries_ingress++;
 link->total_deliveries++;
 }
+//
+// There is no address that we can send this delivery to, which means 
the addr was not found in our hastable. This
+// can be because there were no receivers or because the address was 
not defined in the config file.
+// If the treatment for such addresses is set to be forbidden, we send 
back a rejected disposition and detach the link
+//
+else if (core->qd->treatment == QD_TREATMENT_LINK_FORBIDDEN) {
+dlv->disposition = PN_REJECTED;
+dlv->error = qdr_error("qd:forbidden", "Sending deliveries to this 
address is forbidden");
--- End diff --

You should be using one of the AMQP-specified error conditions that is in 
the amqp.[ch] files.  It seems that the appropriate error condition is 
"amqp:not-allowed", not "forbidden" (which is an error code only for the 
management protocol).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ted-ross
Github user ted-ross commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/185#discussion_r132260270
  
--- Diff: src/router_core/connections.c ---
@@ -1300,9 +1310,15 @@ static void 
qdr_link_inbound_first_attach_CT(qdr_core_t *core, qdr_action_t *act
 //
 // This link has a target address
 //
-bool   link_route;
-qdr_address_t *addr = qdr_lookup_terminus_address_CT(core, 
dir, conn, target, true, true, _route);
-if (!addr) {
+bool  link_route;
+bool  forbidden;
+qdr_address_t *addr = qdr_lookup_terminus_address_CT(core, 
dir, conn, target, true, true, _route, );
+if (forbidden) {
+qdr_link_outbound_detach_CT(core, link, 0, 
QDR_CONDITION_FORBIDDEN, true);
--- End diff --

Does this do the right thing protocol-wise?  Does the router send an attach 
with NULL terminus followed by a detach?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #185: DISPATCH-803 - The following changes were m...

2017-08-09 Thread ganeshmurthy
GitHub user ganeshmurthy opened a pull request:

https://github.com/apache/qpid-dispatch/pull/185

DISPATCH-803 - The following changes were made to support forbidden d…

…istribution

1. Added a new attribute to the router entity called called 
defaultDistribution which defaults to balanced
but can be set to forbidden
2. Attaches to forbidden addresses are rejected and the link is detached
3. Anonymous senders sending to forbidden addresses will be sent back a 
disposition of PN_REJECTED but link will not be closed
4. Added system test system_tests_default_distribution.py to test the above 
cases

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ganeshmurthy/qpid-dispatch DISPATCH-803

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/qpid-dispatch/pull/185.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #185


commit 6ddee464d11e570fdfe8f2599e1587c3f4fc0c42
Author: Ganesh Murthy 
Date:   2017-08-09T17:41:08Z

DISPATCH-803 - The following changes were made to support forbidden 
distribution
1. Added a new attribute to the router entity called called 
defaultDistribution which defaults to balanced
but can be set to forbidden
2. Attaches to forbidden addresses are rejected and the link is detached
3. Anonymous senders sending to forbidden addresses will be sent back a 
disposition of PN_REJECTED but link will not be closed
4. Added system test system_tests_default_distribution.py to test the above 
cases




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org